From 8a9d3d45712d2d142310db81980a1fac11ac238b Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Wed, 8 Nov 2023 21:37:16 +0000 Subject: [PATCH 1/6] feat: add `MaxDepth` for recursive queries --- .../MapperAttribute.cs | 5 + .../MapperMaxRecursionDepthAttribute.cs | 25 +++ .../PublicAPI.Shipped.txt | 5 + src/Riok.Mapperly/AnalyzerReleases.Shipped.md | 1 + .../Configuration/MapperConfiguration.cs | 5 + .../MapperConfigurationMerger.cs | 3 + .../MapperConfigurationReader.cs | 9 +- .../MembersMappingConfiguration.cs | 3 +- .../InlineExpressionMappingBuilderContext.cs | 9 + .../Descriptors/MappingBuilderContext.cs | 5 + .../MappingRecursionDepthTracker.cs | 42 ++++ .../Mappings/DefaultMemberMapping.cs | 16 ++ .../Diagnostics/DiagnosticDescriptors.cs | 9 + .../Mapping/QueryableProjectionLoopTest.cs | 197 ++++++++++++++++++ .../Mapping/QueryableProjectionTest.cs | 33 ++- test/Riok.Mapperly.Tests/TestSourceBuilder.cs | 4 + .../TestSourceBuilderOptions.cs | 3 +- ...lyDefaultShouldWork#MyMapper.g.verified.cs | 22 ++ ...rideAssemblyDefault#MyMapper.g.verified.cs | 28 +++ ...IndirectReferenceLoop#Mapper.g.verified.cs | 76 +++++++ ...RecursionDepthForLoop#Mapper.g.verified.cs | 37 ++++ ...RecursionDepthForLoop#Mapper.g.verified.cs | 37 ++++ ...est.ReferenceLoopCtor#Mapper.g.verified.cs | 13 ++ ...renceLoopInitProperty#Mapper.g.verified.cs | 40 ++++ ...etRecursionDepthToOne#Mapper.g.verified.cs | 19 ++ ...etRecursionDepthToTwo#Mapper.g.verified.cs | 22 ++ ...RecursionDepthToZero#Mapper.g.verified.cs} | 0 ...etRecursiveDepthToOne#Mapper.g.verified.cs | 18 ++ ...etRecursiveDepthToTwo#Mapper.g.verified.cs | 21 ++ ...RecursiveDepthToZero#Mapper.g.verified.cs} | 0 30 files changed, 686 insertions(+), 21 deletions(-) create mode 100644 src/Riok.Mapperly.Abstractions/MapperMaxRecursionDepthAttribute.cs create mode 100644 src/Riok.Mapperly/Descriptors/MappingRecursionDepthTracker.cs create mode 100644 src/Riok.Mapperly/Descriptors/Mappings/DefaultMemberMapping.cs create mode 100644 test/Riok.Mapperly.Tests/Mapping/QueryableProjectionLoopTest.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.AssemblyDefaultShouldWork#MyMapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.AttributeShouldOverrideAssemblyDefault#MyMapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.IndirectReferenceLoop#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.MethodAttributeOverridesClassSetRecursionDepthForLoop#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.MethodAttributeSetRecursionDepthForLoop#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.ReferenceLoopCtor#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.ReferenceLoopInitProperty#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToOne#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToTwo#Mapper.g.verified.cs rename test/Riok.Mapperly.Tests/_snapshots/{QueryableProjectionTest.ReferenceLoopInitProperty#Mapper.g.verified.cs => QueryableProjectionLoopTest.SetRecursionDepthToZero#Mapper.g.verified.cs} (100%) create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToOne#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToTwo#Mapper.g.verified.cs rename test/Riok.Mapperly.Tests/_snapshots/{QueryableProjectionTest.ReferenceLoopCtor#Mapper.g.verified.cs => QueryableProjectionLoopTest.SetRecursiveDepthToZero#Mapper.g.verified.cs} (100%) diff --git a/src/Riok.Mapperly.Abstractions/MapperAttribute.cs b/src/Riok.Mapperly.Abstractions/MapperAttribute.cs index 3a129163bc..c56c5f606c 100644 --- a/src/Riok.Mapperly.Abstractions/MapperAttribute.cs +++ b/src/Riok.Mapperly.Abstractions/MapperAttribute.cs @@ -120,4 +120,9 @@ public class MapperAttribute : Attribute /// partial methods are discovered. /// public bool AutoUserMappings { get; set; } = true; + + /// + /// Defines the maximum recursion depth that an IQueryable mapping will use. + /// + public int MaxRecursionDepth { get; set; } = 8; } diff --git a/src/Riok.Mapperly.Abstractions/MapperMaxRecursionDepthAttribute.cs b/src/Riok.Mapperly.Abstractions/MapperMaxRecursionDepthAttribute.cs new file mode 100644 index 0000000000..5547d94b7a --- /dev/null +++ b/src/Riok.Mapperly.Abstractions/MapperMaxRecursionDepthAttribute.cs @@ -0,0 +1,25 @@ +using System.Diagnostics; + +namespace Riok.Mapperly.Abstractions; + +/// +/// Defines the maximum recursion depth that an IQueryable mapping will use. +/// +[AttributeUsage(AttributeTargets.Method)] +[Conditional("MAPPERLY_ABSTRACTIONS_SCOPE_RUNTIME")] +public sealed class MapperMaxRecursionDepthAttribute : Attribute +{ + /// + /// Defines the maximum recursion depth that an IQueryable mapping will use. + /// + /// The maximum recursion depth used when mapping IQueryable members. + public MapperMaxRecursionDepthAttribute(int maxRecursionDepth) + { + MaxRecursionDepth = maxRecursionDepth; + } + + /// + /// The maximum recursion depth used when mapping IQueryable members. + /// + public int MaxRecursionDepth { get; } +} diff --git a/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt b/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt index f1caebc197..fe313df000 100644 --- a/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt +++ b/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt @@ -155,3 +155,8 @@ Riok.Mapperly.Abstractions.UserMappingAttribute.Default.get -> bool Riok.Mapperly.Abstractions.UserMappingAttribute.Default.set -> void Riok.Mapperly.Abstractions.MapPropertyAttribute.Use.get -> string? Riok.Mapperly.Abstractions.MapPropertyAttribute.Use.set -> void +Riok.Mapperly.Abstractions.MapperAttribute.MaxRecursionDepth.get -> int +Riok.Mapperly.Abstractions.MapperAttribute.MaxRecursionDepth.set -> void +Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute +Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute.MapperMaxRecursionDepthAttribute(int maxRecursionDepth) -> void +Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute.MaxRecursionDepth.get -> int diff --git a/src/Riok.Mapperly/AnalyzerReleases.Shipped.md b/src/Riok.Mapperly/AnalyzerReleases.Shipped.md index 44006c8c6e..ca00c1b0a2 100644 --- a/src/Riok.Mapperly/AnalyzerReleases.Shipped.md +++ b/src/Riok.Mapperly/AnalyzerReleases.Shipped.md @@ -131,6 +131,7 @@ RMG055 | Mapper | Error | The source type does not implement ToString with RMG056 | Mapper | Error | Invalid format provider signature RMG057 | Mapper | Error | Format provider not found RMG058 | Mapper | Error | Multiple default format providers found, only one is allowed +RMG059 | Mapper | Error | The value of MaxRecursionDepth cannot be less than zero ### Removed Rules Rule ID | Category | Severity | Notes diff --git a/src/Riok.Mapperly/Configuration/MapperConfiguration.cs b/src/Riok.Mapperly/Configuration/MapperConfiguration.cs index a82b9a80dc..f3628660c8 100644 --- a/src/Riok.Mapperly/Configuration/MapperConfiguration.cs +++ b/src/Riok.Mapperly/Configuration/MapperConfiguration.cs @@ -115,4 +115,9 @@ public record MapperConfiguration /// Whether to consider non-partial methods in a mapper as user implemented mapping methods. /// public bool? AutoUserMappings { get; init; } + + /// + /// Defines the maximum recursion depth that an IQueryable mapping will use. + /// + public int? MaxRecursionDepth { get; init; } } diff --git a/src/Riok.Mapperly/Configuration/MapperConfigurationMerger.cs b/src/Riok.Mapperly/Configuration/MapperConfigurationMerger.cs index d85911f016..fa53c68316 100644 --- a/src/Riok.Mapperly/Configuration/MapperConfigurationMerger.cs +++ b/src/Riok.Mapperly/Configuration/MapperConfigurationMerger.cs @@ -62,6 +62,9 @@ public static MapperAttribute Merge(MapperConfiguration mapperConfiguration, Map mapper.AutoUserMappings = mapperConfiguration.AutoUserMappings ?? defaultMapperConfiguration.AutoUserMappings ?? mapper.AutoUserMappings; + mapper.MaxRecursionDepth = + mapperConfiguration.MaxRecursionDepth ?? defaultMapperConfiguration.MaxRecursionDepth ?? mapper.MaxRecursionDepth; + return mapper; } } diff --git a/src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs b/src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs index 31596f4fbc..c1e5e111b2 100644 --- a/src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs +++ b/src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs @@ -39,7 +39,8 @@ MapperConfiguration defaultMapperConfiguration Array.Empty(), Array.Empty(), mapper.IgnoreObsoleteMembersStrategy, - mapper.RequiredMappingStrategy + mapper.RequiredMappingStrategy, + mapper.MaxRecursionDepth ), Array.Empty() ); @@ -86,6 +87,12 @@ private MembersMappingConfiguration BuildMembersConfig(MappingConfigurationRefer .AccessFirstOrDefault(configRef.Method) ?.IgnoreObsoleteStrategy; var requiredMapping = _dataAccessor.AccessFirstOrDefault(configRef.Method)?.RequiredMappingStrategy; + var maxRecursionDepth = _dataAccessor.Access(method).FirstOrDefault() + is not { } methodMaxRecursionDepth + ? _defaultConfiguration.Properties.MaxRecursionDepth + : methodMaxRecursionDepth.MaxRecursionDepth; + + // ignore the required mapping / ignore obsolete as the same attribute is used for other mapping types // e.g. enum to enum diff --git a/src/Riok.Mapperly/Configuration/MembersMappingConfiguration.cs b/src/Riok.Mapperly/Configuration/MembersMappingConfiguration.cs index 0f8f9d9dc7..dee254b5b5 100644 --- a/src/Riok.Mapperly/Configuration/MembersMappingConfiguration.cs +++ b/src/Riok.Mapperly/Configuration/MembersMappingConfiguration.cs @@ -7,5 +7,6 @@ public record MembersMappingConfiguration( IReadOnlyCollection IgnoredTargets, IReadOnlyCollection ExplicitMappings, IgnoreObsoleteMembersStrategy IgnoreObsoleteMembersStrategy, - RequiredMappingStrategy RequiredMappingStrategy + RequiredMappingStrategy RequiredMappingStrategy, + int MaxRecursionDepth ); diff --git a/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs b/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs index 0257c7faa7..cc43e389c0 100644 --- a/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs +++ b/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs @@ -91,6 +91,15 @@ conversionType is not MappingConversionType.EnumToString and not MappingConversi public INewInstanceMapping? FindNewInstanceMapping(IMethodSymbol method) { + // check for recursion loop returning null to prevent a loop or default when recursion limit is reached. + var count = _parentTypes.GetDepth(mappingKey); + if (count >= 1) + { + return count >= Configuration.Properties.MaxRecursionDepth + 2 + ? new DefaultMemberMapping(mappingKey.Source, mappingKey.Target) + : null; + } + INewInstanceMapping? mapping = InlinedMappings.FindNewInstanceUserMapping(method, out var isInlined); if (mapping == null) return null; diff --git a/src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs b/src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs index 50e3b5a970..565e53436c 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs @@ -18,6 +18,7 @@ public class MappingBuilderContext : SimpleMappingBuilderContext private readonly FormatProviderCollection _formatProviders; private CollectionInfos? _collectionInfos; private DictionaryInfos? _dictionaryInfos; + internal readonly MappingRecursionDepthTracker _parentTypes; public MappingBuilderContext( SimpleMappingBuilderContext parentCtx, @@ -31,6 +32,10 @@ public MappingBuilderContext( { ObjectFactories = objectFactories; _formatProviders = formatProviders; + _parentTypes = parentCtx is MappingBuilderContext inlineCtx + ? inlineCtx._parentTypes.AddOrIncrement(mappingKey) + : MappingRecursionDepthTracker.Create(mappingKey); + UserSymbol = userSymbol; MappingKey = mappingKey; Configuration = ReadConfiguration(new MappingConfigurationReference(UserSymbol, mappingKey.Source, mappingKey.Target)); diff --git a/src/Riok.Mapperly/Descriptors/MappingRecursionDepthTracker.cs b/src/Riok.Mapperly/Descriptors/MappingRecursionDepthTracker.cs new file mode 100644 index 0000000000..371220ae54 --- /dev/null +++ b/src/Riok.Mapperly/Descriptors/MappingRecursionDepthTracker.cs @@ -0,0 +1,42 @@ +using System.Collections.Immutable; + +namespace Riok.Mapperly.Descriptors; + +/// +/// Immutable wrapper for which tracks the parent types for a mapping. +/// Used to detect self referential loops. +/// +/// Dictionary tracking how many times a type has been seen. +public readonly struct MappingRecursionDepthTracker(ImmutableDictionary parentTypes) +{ + /// + /// Increments how many times a has been mapped. + /// Used to track how many times a parent context has mapped a type. + /// + /// The mapped type. + /// A new with the updated key. + public MappingRecursionDepthTracker AddOrIncrement(TypeMappingKey typeMappingKey) + { + var mappingRecursionCount = parentTypes.GetValueOrDefault(typeMappingKey); + var newParentTypes = parentTypes.SetItem(typeMappingKey, mappingRecursionCount + 1); + return new(newParentTypes); + } + + /// + /// Gets the number of times a has been mapped by the parent contexts. + /// + /// The candidate mapping. + /// The number of times the has been mapped. + public int GetDepth(TypeMappingKey typeMappingKey) => parentTypes.GetValueOrDefault(typeMappingKey); + + /// + /// Creates a new containing the initial type mapping. + /// + /// Initial value. + /// A containing the initial type mapping. + public static MappingRecursionDepthTracker Create(TypeMappingKey mappingKey) + { + var dict = ImmutableDictionary.Empty; + return new(dict.Add(mappingKey, 1)); + } +} diff --git a/src/Riok.Mapperly/Descriptors/Mappings/DefaultMemberMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/DefaultMemberMapping.cs new file mode 100644 index 0000000000..1fe857a806 --- /dev/null +++ b/src/Riok.Mapperly/Descriptors/Mappings/DefaultMemberMapping.cs @@ -0,0 +1,16 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using static Riok.Mapperly.Emit.Syntax.SyntaxFactoryHelper; + +namespace Riok.Mapperly.Descriptors.Mappings; + +/// +/// Represents a mapping that returns default. +/// +/// target = default; +/// +/// +public class DefaultMemberMapping(ITypeSymbol sourceType, ITypeSymbol targetType) : NewInstanceMapping(sourceType, targetType) +{ + public override ExpressionSyntax Build(TypeMappingBuildContext ctx) => DefaultLiteral(); +} diff --git a/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs b/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs index a790c1f908..2640ad098e 100644 --- a/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs +++ b/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs @@ -667,6 +667,15 @@ public static class DiagnosticDescriptors true ); + public static readonly DiagnosticDescriptor MaxRecursionDepthMustBeZeroOrMore = new DiagnosticDescriptor( + "RMG0569", + $"The value of MaxRecursionDepth cannot be less than zero", + $"The value of MaxRecursionDepth cannot be less than zero", + DiagnosticCategories.Mapper, + DiagnosticSeverity.Error, + true + ); + private static string BuildHelpUri(string id) { #if ENV_NEXT diff --git a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionLoopTest.cs b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionLoopTest.cs new file mode 100644 index 0000000000..17c368f97e --- /dev/null +++ b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionLoopTest.cs @@ -0,0 +1,197 @@ +using Riok.Mapperly.Diagnostics; + +namespace Riok.Mapperly.Tests.Mapping; + +[UsesVerify] +public class QueryableProjectionLoopTest +{ + [Fact] + public Task ReferenceLoopInitProperty() + { + var source = TestSourceBuilder.Mapping( + "System.Linq.IQueryable", + "System.Linq.IQueryable", + "class A { public A? Parent { get; set; } }", + "class B { public B? Parent { get; set; } }" + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task SetRecursionDepthToZero() + { + var source = TestSourceBuilder.Mapping( + "System.Linq.IQueryable", + "System.Linq.IQueryable", + TestSourceBuilderOptions.Default with + { + MaxRecursionDepth = 0 + }, + "class A { public A? Parent { get; set; } }", + "class B { public B? Parent { get; set; } }" + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task SetRecursionDepthToOne() + { + var source = TestSourceBuilder.Mapping( + "System.Linq.IQueryable", + "System.Linq.IQueryable", + TestSourceBuilderOptions.Default with + { + MaxRecursionDepth = 1 + }, + "class A { public A? Parent { get; set; } }", + "class B { public B? Parent { get; set; } }" + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task SetRecursionDepthToTwo() + { + var source = TestSourceBuilder.Mapping( + "System.Linq.IQueryable", + "System.Linq.IQueryable", + TestSourceBuilderOptions.Default with + { + MaxRecursionDepth = 2 + }, + "class A { public A? Parent { get; set; } }", + "class B { public B? Parent { get; set; } }" + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task MethodAttributeSetRecursionDepthForLoop() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + partial System.Linq.IQueryable Map(System.Linq.IQueryable src); + + [MapperMaxRecursionDepth(2)] + partial B Map(A src); + """, + "class A { public A? Parent { get; set; } }", + "class B { public B? Parent { get; set; } }" + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task MethodAttributeOverridesClassSetRecursionDepthForLoop() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + partial System.Linq.IQueryable Map(System.Linq.IQueryable src); + [MapperMaxRecursionDepth(2)] partial B Map(A src); + """, + TestSourceBuilderOptions.Default with + { + MaxRecursionDepth = 4 + }, + "class A { public A? Parent { get; set; } }", + "class B { public B? Parent { get; set; } }" + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task AssemblyDefaultShouldWork() + { + var source = TestSourceBuilder.CSharp( + """ + using Riok.Mapperly.Abstractions; + + [assembly: MapperDefaultsAttribute(MaxRecursionDepth = 2)] + [Mapper()] + public partial class MyMapper + { + partial System.Linq.IQueryable Map(System.Linq.IQueryable src); + } + + class A { public A? Parent { get; set; } } + + class B { public B? Parent { get; set; } } + """ + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task AttributeShouldOverrideAssemblyDefault() + { + var source = TestSourceBuilder.CSharp( + """ + using Riok.Mapperly.Abstractions; + + [assembly: MapperDefaultsAttribute(MaxRecursionDepth = 2)] + [Mapper(MaxRecursionDepth = 4)] + public partial class MyMapper + { + partial System.Linq.IQueryable Map(System.Linq.IQueryable src); + } + + class A { public A? Parent { get; set; } } + + class B { public B? Parent { get; set; } } + """ + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task ReferenceLoopCtor() + { + var source = TestSourceBuilder.Mapping( + "System.Linq.IQueryable", + "System.Linq.IQueryable", + "class A { public A? Parent { get; set; } }", + "class B { public B(B? parent) {} }" + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task IndirectReferenceLoop() + { + var source = TestSourceBuilder.Mapping( + "System.Linq.IQueryable", + "System.Linq.IQueryable", + "class A { public string StringValue { get; set; } public C Parent { get; set; } }", + "class B { public string StringValue { get; set; } public D Parent { get; set; } }", + "class C { public A Parent { get; set; } }", + "class D { public B Parent { get; set; } }" + ); + + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public void WithReferenceHandlingShouldDiagnostic() + { + var source = TestSourceBuilder.Mapping( + "System.Linq.IQueryable", + "System.Linq.IQueryable", + TestSourceBuilderOptions.WithReferenceHandling + ); + + TestHelper + .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .Should() + .HaveDiagnostic(DiagnosticDescriptors.QueryableProjectionMappingsDoNotSupportReferenceHandling) + .HaveAssertedAllDiagnostics(); + } +} diff --git a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs index 88f23f41e0..f8fa59ad0b 100644 --- a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs @@ -1,5 +1,3 @@ -using Riok.Mapperly.Diagnostics; - namespace Riok.Mapperly.Tests.Mapping; public class QueryableProjectionTest @@ -118,6 +116,21 @@ public Task RecordToRecordManualListMapping() return TestHelper.VerifyGenerator(source); } + [Fact] + public Task ClassToClassWithUserImplemented() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + private partial System.Linq.IQueryable Map(System.Linq.IQueryable source); + + private D MapToD(C v) => new D { Value = v.Value + "-mapped" }; + """, + "class A { public string StringValue { get; set; } public C NestedValue { get; set; } }", + "class B { public string StringValue { get; set; } public D NestedValue { get; set; } }", + "class C { public string Value { get; set; } }", + "class D { public string Value { get; set; } }" + ); + [Fact] public Task ReferenceLoopInitProperty() { @@ -156,20 +169,4 @@ public Task CtorShouldSkipUnmatchedOptionalParameters() return TestHelper.VerifyGenerator(source); } - - [Fact] - public void WithReferenceHandlingShouldDiagnostic() - { - var source = TestSourceBuilder.Mapping( - "System.Linq.IQueryable", - "System.Linq.IQueryable", - TestSourceBuilderOptions.WithReferenceHandling - ); - - TestHelper - .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) - .Should() - .HaveDiagnostic(DiagnosticDescriptors.QueryableProjectionMappingsDoNotSupportReferenceHandling) - .HaveAssertedAllDiagnostics(); - } } diff --git a/test/Riok.Mapperly.Tests/TestSourceBuilder.cs b/test/Riok.Mapperly.Tests/TestSourceBuilder.cs index 9d393a31c6..6cb60d5fe2 100644 --- a/test/Riok.Mapperly.Tests/TestSourceBuilder.cs +++ b/test/Riok.Mapperly.Tests/TestSourceBuilder.cs @@ -97,6 +97,7 @@ private static string BuildAttribute(TestSourceBuilderOptions options) Attribute(options.IncludedMembers), Attribute(options.PreferParameterlessConstructors), Attribute(options.AutoUserMappings), + Attribute(options.MaxRecursionDepth), }.WhereNotNull(); return $"[Mapper({string.Join(", ", attrs)})]"; @@ -114,6 +115,9 @@ private static string BuildAttribute(TestSourceBuilderOptions options) private static string? Attribute(bool? value, [CallerArgumentExpression("value")] string? expression = null) => value.HasValue ? Attribute(value.Value ? "true" : "false", expression) : null; + private static string? Attribute(int? value, [CallerArgumentExpression("value")] string? expression = null) => + value.HasValue ? Attribute(value.Value.ToString(), expression) : null; + private static string? Attribute(string? value, [CallerArgumentExpression("value")] string? expression = null) { if (value == null) diff --git a/test/Riok.Mapperly.Tests/TestSourceBuilderOptions.cs b/test/Riok.Mapperly.Tests/TestSourceBuilderOptions.cs index 43f6431b67..41a1575ed0 100644 --- a/test/Riok.Mapperly.Tests/TestSourceBuilderOptions.cs +++ b/test/Riok.Mapperly.Tests/TestSourceBuilderOptions.cs @@ -20,7 +20,8 @@ public record TestSourceBuilderOptions( MemberVisibility? IncludedMembers = null, bool Static = false, bool PreferParameterlessConstructors = true, - bool AutoUserMappings = true + bool AutoUserMappings = true, + int? MaxRecursionDepth = null ) { public const string DefaultMapperClassName = "Mapper"; diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.AssemblyDefaultShouldWork#MyMapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.AssemblyDefaultShouldWork#MyMapper.g.verified.cs new file mode 100644 index 0000000000..600cd5cfcf --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.AssemblyDefaultShouldWork#MyMapper.g.verified.cs @@ -0,0 +1,22 @@ +//HintName: MyMapper.g.cs +// +#nullable enable +public partial class MyMapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable src) + { +#nullable disable + return System.Linq.Queryable.Select(src, x => new global::B() + { + Parent = x.Parent != null ? new global::B() + { + Parent = x.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent != null ? default : default, + } : default, + } : default, + }); +#nullable enable + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.AttributeShouldOverrideAssemblyDefault#MyMapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.AttributeShouldOverrideAssemblyDefault#MyMapper.g.verified.cs new file mode 100644 index 0000000000..7376df097d --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.AttributeShouldOverrideAssemblyDefault#MyMapper.g.verified.cs @@ -0,0 +1,28 @@ +//HintName: MyMapper.g.cs +// +#nullable enable +public partial class MyMapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable src) + { +#nullable disable + return System.Linq.Queryable.Select(src, x => new global::B() + { + Parent = x.Parent != null ? new global::B() + { + Parent = x.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent.Parent.Parent != null ? default : default, + } : default, + } : default, + } : default, + } : default, + }); +#nullable enable + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.IndirectReferenceLoop#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.IndirectReferenceLoop#Mapper.g.verified.cs new file mode 100644 index 0000000000..b2d67ec446 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.IndirectReferenceLoop#Mapper.g.verified.cs @@ -0,0 +1,76 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + private partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable source) + { +#nullable disable + return System.Linq.Queryable.Select(source, x => new global::B() + { + StringValue = x.StringValue, + Parent = new global::D() + { + Parent = new global::B() + { + StringValue = x.Parent.Parent.StringValue, + Parent = new global::D() + { + Parent = new global::B() + { + StringValue = x.Parent.Parent.Parent.Parent.StringValue, + Parent = new global::D() + { + Parent = new global::B() + { + StringValue = x.Parent.Parent.Parent.Parent.Parent.Parent.StringValue, + Parent = new global::D() + { + Parent = new global::B() + { + StringValue = x.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.StringValue, + Parent = new global::D() + { + Parent = new global::B() + { + StringValue = x.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.StringValue, + Parent = new global::D() + { + Parent = new global::B() + { + StringValue = x.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.StringValue, + Parent = new global::D() + { + Parent = new global::B() + { + StringValue = x.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.StringValue, + Parent = new global::D() + { + Parent = new global::B() + { + StringValue = x.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.StringValue, + Parent = new global::D() + { + Parent = default, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }); +#nullable enable + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.MethodAttributeOverridesClassSetRecursionDepthForLoop#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.MethodAttributeOverridesClassSetRecursionDepthForLoop#Mapper.g.verified.cs new file mode 100644 index 0000000000..ed584c10f1 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.MethodAttributeOverridesClassSetRecursionDepthForLoop#Mapper.g.verified.cs @@ -0,0 +1,37 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable src) + { +#nullable disable + return System.Linq.Queryable.Select(src, x => new global::B() + { + Parent = x.Parent != null ? new global::B() + { + Parent = x.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent != null ? default : default, + } : default, + } : default, + }); +#nullable enable + } + + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + partial global::B Map(global::A src) + { + var target = new global::B(); + if (src.Parent != null) + { + target.Parent = Map(src.Parent); + } + else + { + target.Parent = null; + } + return target; + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.MethodAttributeSetRecursionDepthForLoop#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.MethodAttributeSetRecursionDepthForLoop#Mapper.g.verified.cs new file mode 100644 index 0000000000..ed584c10f1 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.MethodAttributeSetRecursionDepthForLoop#Mapper.g.verified.cs @@ -0,0 +1,37 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable src) + { +#nullable disable + return System.Linq.Queryable.Select(src, x => new global::B() + { + Parent = x.Parent != null ? new global::B() + { + Parent = x.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent != null ? default : default, + } : default, + } : default, + }); +#nullable enable + } + + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + partial global::B Map(global::A src) + { + var target = new global::B(); + if (src.Parent != null) + { + target.Parent = Map(src.Parent); + } + else + { + target.Parent = null; + } + return target; + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.ReferenceLoopCtor#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.ReferenceLoopCtor#Mapper.g.verified.cs new file mode 100644 index 0000000000..077fbd8ef2 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.ReferenceLoopCtor#Mapper.g.verified.cs @@ -0,0 +1,13 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + private partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable source) + { +#nullable disable + return System.Linq.Queryable.Select(source, x => new global::B(x.Parent != null ? new global::B(x.Parent.Parent != null ? new global::B(x.Parent.Parent.Parent != null ? new global::B(x.Parent.Parent.Parent.Parent != null ? new global::B(x.Parent.Parent.Parent.Parent.Parent != null ? new global::B(x.Parent.Parent.Parent.Parent.Parent.Parent != null ? new global::B(x.Parent.Parent.Parent.Parent.Parent.Parent.Parent != null ? new global::B(x.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent != null ? new global::B(x.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent != null ? default : default) : default) : default) : default) : default) : default) : default) : default) : default)); +#nullable enable + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.ReferenceLoopInitProperty#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.ReferenceLoopInitProperty#Mapper.g.verified.cs new file mode 100644 index 0000000000..58f0cc8d06 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.ReferenceLoopInitProperty#Mapper.g.verified.cs @@ -0,0 +1,40 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + private partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable source) + { +#nullable disable + return System.Linq.Queryable.Select(source, x => new global::B() + { + Parent = x.Parent != null ? new global::B() + { + Parent = x.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent.Parent.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent.Parent.Parent.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent != null ? default : default, + } : default, + } : default, + } : default, + } : default, + } : default, + } : default, + } : default, + } : default, + }); +#nullable enable + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToOne#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToOne#Mapper.g.verified.cs new file mode 100644 index 0000000000..5d88c750fc --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToOne#Mapper.g.verified.cs @@ -0,0 +1,19 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + private partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable source) + { +#nullable disable + return System.Linq.Queryable.Select(source, x => new global::B() + { + Parent = x.Parent != null ? new global::B() + { + Parent = x.Parent.Parent != null ? default : default, + } : default, + }); +#nullable enable + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToTwo#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToTwo#Mapper.g.verified.cs new file mode 100644 index 0000000000..0a3ee11d1c --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToTwo#Mapper.g.verified.cs @@ -0,0 +1,22 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + private partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable source) + { +#nullable disable + return System.Linq.Queryable.Select(source, x => new global::B() + { + Parent = x.Parent != null ? new global::B() + { + Parent = x.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent != null ? default : default, + } : default, + } : default, + }); +#nullable enable + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionTest.ReferenceLoopInitProperty#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToZero#Mapper.g.verified.cs similarity index 100% rename from test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionTest.ReferenceLoopInitProperty#Mapper.g.verified.cs rename to test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToZero#Mapper.g.verified.cs diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToOne#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToOne#Mapper.g.verified.cs new file mode 100644 index 0000000000..49888db769 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToOne#Mapper.g.verified.cs @@ -0,0 +1,18 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + private partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable source) + { +#nullable disable + return System.Linq.Queryable.Select(source, x => new global::B() + { + Parent = x.Parent != null ? new global::B() + { + Parent = x.Parent.Parent != null ? default : default, + } : default, + }); +#nullable enable + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToTwo#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToTwo#Mapper.g.verified.cs new file mode 100644 index 0000000000..f2f5121794 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToTwo#Mapper.g.verified.cs @@ -0,0 +1,21 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + private partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable source) + { +#nullable disable + return System.Linq.Queryable.Select(source, x => new global::B() + { + Parent = x.Parent != null ? new global::B() + { + Parent = x.Parent.Parent != null ? new global::B() + { + Parent = x.Parent.Parent.Parent != null ? default : default, + } : default, + } : default, + }); +#nullable enable + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionTest.ReferenceLoopCtor#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToZero#Mapper.g.verified.cs similarity index 100% rename from test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionTest.ReferenceLoopCtor#Mapper.g.verified.cs rename to test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursiveDepthToZero#Mapper.g.verified.cs From c7ccc75a3b6f0517edfb2d55e7a02be8c0cb7fee Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Thu, 28 Mar 2024 21:05:38 +0000 Subject: [PATCH 2/6] chore: semi fix --- .../MapperConfigurationReader.cs | 10 +--- .../InlineExpressionMappingBuilderContext.cs | 25 ++++++--- .../Helpers/MapperConfigurationBuilderTest.cs | 3 + .../Mapping/QueryableProjectionLoopTest.cs | 1 - .../Mapping/QueryableProjectionTest.cs | 55 +++++-------------- 5 files changed, 38 insertions(+), 56 deletions(-) diff --git a/src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs b/src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs index c1e5e111b2..c4a80aa446 100644 --- a/src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs +++ b/src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs @@ -87,12 +87,7 @@ private MembersMappingConfiguration BuildMembersConfig(MappingConfigurationRefer .AccessFirstOrDefault(configRef.Method) ?.IgnoreObsoleteStrategy; var requiredMapping = _dataAccessor.AccessFirstOrDefault(configRef.Method)?.RequiredMappingStrategy; - var maxRecursionDepth = _dataAccessor.Access(method).FirstOrDefault() - is not { } methodMaxRecursionDepth - ? _defaultConfiguration.Properties.MaxRecursionDepth - : methodMaxRecursionDepth.MaxRecursionDepth; - - + var maxRecursionDepth = _dataAccessor.AccessFirstOrDefault(configRef.Method)?.MaxRecursionDepth; // ignore the required mapping / ignore obsolete as the same attribute is used for other mapping types // e.g. enum to enum @@ -123,7 +118,8 @@ is not { } methodMaxRecursionDepth ignoredTargetMembers, memberConfigurations, ignoreObsolete ?? MapperConfiguration.Members.IgnoreObsoleteMembersStrategy, - requiredMapping ?? MapperConfiguration.Members.RequiredMappingStrategy + requiredMapping ?? MapperConfiguration.Members.RequiredMappingStrategy, + maxRecursionDepth ?? MapperConfiguration.Members.MaxRecursionDepth ); } diff --git a/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs b/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs index cc43e389c0..73edad4327 100644 --- a/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs +++ b/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs @@ -77,6 +77,15 @@ conversionType is not MappingConversionType.EnumToString and not MappingConversi /// The if a mapping was found or null if none was found. public override INewInstanceMapping? FindMapping(TypeMappingKey mappingKey) { + // check for recursion loop returning null to prevent a loop or default when recursion limit is reached. + var count = _parentTypes.GetDepth(mappingKey); + if (count >= 1) + { + return count >= Configuration.Members.MaxRecursionDepth + 2 + ? new DefaultMemberMapping(mappingKey.Source, mappingKey.Target) + : null; + } + var mapping = InlinedMappings.Find(mappingKey, out var isInlined); if (mapping == null) return null; @@ -91,14 +100,14 @@ conversionType is not MappingConversionType.EnumToString and not MappingConversi public INewInstanceMapping? FindNewInstanceMapping(IMethodSymbol method) { - // check for recursion loop returning null to prevent a loop or default when recursion limit is reached. - var count = _parentTypes.GetDepth(mappingKey); - if (count >= 1) - { - return count >= Configuration.Properties.MaxRecursionDepth + 2 - ? new DefaultMemberMapping(mappingKey.Source, mappingKey.Target) - : null; - } + // // check for recursion loop returning null to prevent a loop or default when recursion limit is reached. + // var count = _parentTypes.GetDepth(mappingKey); + // if (count >= 1) + // { + // return count >= Configuration.Properties.MaxRecursionDepth + 2 + // ? new DefaultMemberMapping(mappingKey.Source, mappingKey.Target) + // : null; + // } INewInstanceMapping? mapping = InlinedMappings.FindNewInstanceUserMapping(method, out var isInlined); if (mapping == null) diff --git a/test/Riok.Mapperly.Tests/Helpers/MapperConfigurationBuilderTest.cs b/test/Riok.Mapperly.Tests/Helpers/MapperConfigurationBuilderTest.cs index a9fc4168de..6533975e28 100644 --- a/test/Riok.Mapperly.Tests/Helpers/MapperConfigurationBuilderTest.cs +++ b/test/Riok.Mapperly.Tests/Helpers/MapperConfigurationBuilderTest.cs @@ -94,6 +94,9 @@ private MapperConfiguration NewMapperConfiguration() if (type == typeof(bool)) return !modifiedValue; + if (type == typeof(int)) + return !modifiedValue; + if (type.IsEnum) return type.GetEnumValues().GetValue(modifiedValue ? 1 : 0); diff --git a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionLoopTest.cs b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionLoopTest.cs index 17c368f97e..36a22a79d5 100644 --- a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionLoopTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionLoopTest.cs @@ -2,7 +2,6 @@ namespace Riok.Mapperly.Tests.Mapping; -[UsesVerify] public class QueryableProjectionLoopTest { [Fact] diff --git a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs index f8fa59ad0b..e833374829 100644 --- a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs @@ -116,46 +116,21 @@ public Task RecordToRecordManualListMapping() return TestHelper.VerifyGenerator(source); } - [Fact] - public Task ClassToClassWithUserImplemented() - { - var source = TestSourceBuilder.MapperWithBodyAndTypes( - """ - private partial System.Linq.IQueryable Map(System.Linq.IQueryable source); - - private D MapToD(C v) => new D { Value = v.Value + "-mapped" }; - """, - "class A { public string StringValue { get; set; } public C NestedValue { get; set; } }", - "class B { public string StringValue { get; set; } public D NestedValue { get; set; } }", - "class C { public string Value { get; set; } }", - "class D { public string Value { get; set; } }" - ); - - [Fact] - public Task ReferenceLoopInitProperty() - { - var source = TestSourceBuilder.Mapping( - "System.Linq.IQueryable", - "System.Linq.IQueryable", - "class A { public A? Parent { get; set; } public int IntValue { get; set; } }", - "class B { public B? Parent { get; set; } public int IntValue { get; set; } }" - ); - - return TestHelper.VerifyGenerator(source); - } - - [Fact] - public Task ReferenceLoopCtor() - { - var source = TestSourceBuilder.Mapping( - "System.Linq.IQueryable", - "System.Linq.IQueryable", - "class A { public A? Parent { get; set; } public int IntValue { get; set; } }", - "class B { public B(B? parent) {} public int IntValue { get; set; } }" - ); - - return TestHelper.VerifyGenerator(source); - } + // [Fact] + // public Task ClassToClassWithUserImplemented() + // { + // var source = TestSourceBuilder.MapperWithBodyAndTypes( + // """ + // private partial System.Linq.IQueryable Map(System.Linq.IQueryable source); + // + // private D MapToD(C v) => new D { Value = v.Value + "-mapped" }; + // """, + // "class A { public string StringValue { get; set; } public C NestedValue { get; set; } }", + // "class B { public string StringValue { get; set; } public D NestedValue { get; set; } }", + // "class C { public string Value { get; set; } }", + // "class D { public string Value { get; set; } }" + // ); + // } [Fact] public Task CtorShouldSkipUnmatchedOptionalParameters() From bc56fa250aa700bdda4056f7d258e2ffed2b2d63 Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Fri, 29 Mar 2024 11:55:04 +0000 Subject: [PATCH 3/6] chore: use `uint` --- src/Riok.Mapperly.Abstractions/MapperAttribute.cs | 2 +- .../MapperMaxRecursionDepthAttribute.cs | 4 ++-- src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt | 4 ++-- src/Riok.Mapperly/Configuration/MapperConfiguration.cs | 2 +- .../Configuration/MembersMappingConfiguration.cs | 2 +- src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs | 9 --------- test/Riok.Mapperly.Tests/TestSourceBuilder.cs | 2 +- test/Riok.Mapperly.Tests/TestSourceBuilderOptions.cs | 2 +- ...LoopTest.SetRecursionDepthToZero#Mapper.g.verified.cs | 6 +----- 9 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/Riok.Mapperly.Abstractions/MapperAttribute.cs b/src/Riok.Mapperly.Abstractions/MapperAttribute.cs index c56c5f606c..d8340d70a3 100644 --- a/src/Riok.Mapperly.Abstractions/MapperAttribute.cs +++ b/src/Riok.Mapperly.Abstractions/MapperAttribute.cs @@ -124,5 +124,5 @@ public class MapperAttribute : Attribute /// /// Defines the maximum recursion depth that an IQueryable mapping will use. /// - public int MaxRecursionDepth { get; set; } = 8; + public uint MaxRecursionDepth { get; set; } = 8; } diff --git a/src/Riok.Mapperly.Abstractions/MapperMaxRecursionDepthAttribute.cs b/src/Riok.Mapperly.Abstractions/MapperMaxRecursionDepthAttribute.cs index 5547d94b7a..5171dc064a 100644 --- a/src/Riok.Mapperly.Abstractions/MapperMaxRecursionDepthAttribute.cs +++ b/src/Riok.Mapperly.Abstractions/MapperMaxRecursionDepthAttribute.cs @@ -13,7 +13,7 @@ public sealed class MapperMaxRecursionDepthAttribute : Attribute /// Defines the maximum recursion depth that an IQueryable mapping will use. /// /// The maximum recursion depth used when mapping IQueryable members. - public MapperMaxRecursionDepthAttribute(int maxRecursionDepth) + public MapperMaxRecursionDepthAttribute(uint maxRecursionDepth) { MaxRecursionDepth = maxRecursionDepth; } @@ -21,5 +21,5 @@ public MapperMaxRecursionDepthAttribute(int maxRecursionDepth) /// /// The maximum recursion depth used when mapping IQueryable members. /// - public int MaxRecursionDepth { get; } + public uint MaxRecursionDepth { get; } } diff --git a/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt b/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt index fe313df000..2898b86424 100644 --- a/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt +++ b/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt @@ -158,5 +158,5 @@ Riok.Mapperly.Abstractions.MapPropertyAttribute.Use.set -> void Riok.Mapperly.Abstractions.MapperAttribute.MaxRecursionDepth.get -> int Riok.Mapperly.Abstractions.MapperAttribute.MaxRecursionDepth.set -> void Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute -Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute.MapperMaxRecursionDepthAttribute(int maxRecursionDepth) -> void -Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute.MaxRecursionDepth.get -> int +Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute.MapperMaxRecursionDepthAttribute(uint maxRecursionDepth) -> void +Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute.MaxRecursionDepth.get -> uint diff --git a/src/Riok.Mapperly/Configuration/MapperConfiguration.cs b/src/Riok.Mapperly/Configuration/MapperConfiguration.cs index f3628660c8..74316f5bb2 100644 --- a/src/Riok.Mapperly/Configuration/MapperConfiguration.cs +++ b/src/Riok.Mapperly/Configuration/MapperConfiguration.cs @@ -119,5 +119,5 @@ public record MapperConfiguration /// /// Defines the maximum recursion depth that an IQueryable mapping will use. /// - public int? MaxRecursionDepth { get; init; } + public uint? MaxRecursionDepth { get; init; } } diff --git a/src/Riok.Mapperly/Configuration/MembersMappingConfiguration.cs b/src/Riok.Mapperly/Configuration/MembersMappingConfiguration.cs index dee254b5b5..9025e65eb4 100644 --- a/src/Riok.Mapperly/Configuration/MembersMappingConfiguration.cs +++ b/src/Riok.Mapperly/Configuration/MembersMappingConfiguration.cs @@ -8,5 +8,5 @@ public record MembersMappingConfiguration( IReadOnlyCollection ExplicitMappings, IgnoreObsoleteMembersStrategy IgnoreObsoleteMembersStrategy, RequiredMappingStrategy RequiredMappingStrategy, - int MaxRecursionDepth + uint MaxRecursionDepth ); diff --git a/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs b/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs index 2640ad098e..a790c1f908 100644 --- a/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs +++ b/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs @@ -667,15 +667,6 @@ public static class DiagnosticDescriptors true ); - public static readonly DiagnosticDescriptor MaxRecursionDepthMustBeZeroOrMore = new DiagnosticDescriptor( - "RMG0569", - $"The value of MaxRecursionDepth cannot be less than zero", - $"The value of MaxRecursionDepth cannot be less than zero", - DiagnosticCategories.Mapper, - DiagnosticSeverity.Error, - true - ); - private static string BuildHelpUri(string id) { #if ENV_NEXT diff --git a/test/Riok.Mapperly.Tests/TestSourceBuilder.cs b/test/Riok.Mapperly.Tests/TestSourceBuilder.cs index 6cb60d5fe2..9835e58f73 100644 --- a/test/Riok.Mapperly.Tests/TestSourceBuilder.cs +++ b/test/Riok.Mapperly.Tests/TestSourceBuilder.cs @@ -115,7 +115,7 @@ private static string BuildAttribute(TestSourceBuilderOptions options) private static string? Attribute(bool? value, [CallerArgumentExpression("value")] string? expression = null) => value.HasValue ? Attribute(value.Value ? "true" : "false", expression) : null; - private static string? Attribute(int? value, [CallerArgumentExpression("value")] string? expression = null) => + private static string? Attribute(uint? value, [CallerArgumentExpression("value")] string? expression = null) => value.HasValue ? Attribute(value.Value.ToString(), expression) : null; private static string? Attribute(string? value, [CallerArgumentExpression("value")] string? expression = null) diff --git a/test/Riok.Mapperly.Tests/TestSourceBuilderOptions.cs b/test/Riok.Mapperly.Tests/TestSourceBuilderOptions.cs index 41a1575ed0..4deb968c98 100644 --- a/test/Riok.Mapperly.Tests/TestSourceBuilderOptions.cs +++ b/test/Riok.Mapperly.Tests/TestSourceBuilderOptions.cs @@ -21,7 +21,7 @@ public record TestSourceBuilderOptions( bool Static = false, bool PreferParameterlessConstructors = true, bool AutoUserMappings = true, - int? MaxRecursionDepth = null + uint? MaxRecursionDepth = null ) { public const string DefaultMapperClassName = "Mapper"; diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToZero#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToZero#Mapper.g.verified.cs index 27393fd0b2..a8785be7d6 100644 --- a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToZero#Mapper.g.verified.cs +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionLoopTest.SetRecursionDepthToZero#Mapper.g.verified.cs @@ -9,11 +9,7 @@ public partial class Mapper #nullable disable return System.Linq.Queryable.Select(source, x => new global::B() { - Parent = x.Parent != null ? new global::B() - { - IntValue = x.Parent.IntValue, - } : default, - IntValue = x.IntValue, + Parent = x.Parent != null ? default : default, }); #nullable enable } From 608c7a3f74ee04bb05800ff9efcd65eef8dbbd38 Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Fri, 29 Mar 2024 13:54:24 +0000 Subject: [PATCH 4/6] chore: fix `ShouldMergeMapperConfigurations` --- .../Helpers/MapperConfigurationBuilderTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Riok.Mapperly.Tests/Helpers/MapperConfigurationBuilderTest.cs b/test/Riok.Mapperly.Tests/Helpers/MapperConfigurationBuilderTest.cs index 6533975e28..492d94f171 100644 --- a/test/Riok.Mapperly.Tests/Helpers/MapperConfigurationBuilderTest.cs +++ b/test/Riok.Mapperly.Tests/Helpers/MapperConfigurationBuilderTest.cs @@ -94,8 +94,8 @@ private MapperConfiguration NewMapperConfiguration() if (type == typeof(bool)) return !modifiedValue; - if (type == typeof(int)) - return !modifiedValue; + if (type == typeof(uint)) + return (uint)1; if (type.IsEnum) return type.GetEnumValues().GetValue(modifiedValue ? 1 : 0); From f463113445c471aa327351110394d45d8517af4e Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Fri, 29 Mar 2024 14:13:18 +0000 Subject: [PATCH 5/6] chore: remove unused code --- src/Riok.Mapperly/AnalyzerReleases.Shipped.md | 1 - .../InlineExpressionMappingBuilderContext.cs | 9 --------- .../Mapping/QueryableProjectionTest.cs | 16 ---------------- 3 files changed, 26 deletions(-) diff --git a/src/Riok.Mapperly/AnalyzerReleases.Shipped.md b/src/Riok.Mapperly/AnalyzerReleases.Shipped.md index ca00c1b0a2..44006c8c6e 100644 --- a/src/Riok.Mapperly/AnalyzerReleases.Shipped.md +++ b/src/Riok.Mapperly/AnalyzerReleases.Shipped.md @@ -131,7 +131,6 @@ RMG055 | Mapper | Error | The source type does not implement ToString with RMG056 | Mapper | Error | Invalid format provider signature RMG057 | Mapper | Error | Format provider not found RMG058 | Mapper | Error | Multiple default format providers found, only one is allowed -RMG059 | Mapper | Error | The value of MaxRecursionDepth cannot be less than zero ### Removed Rules Rule ID | Category | Severity | Notes diff --git a/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs b/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs index 73edad4327..18c180f18a 100644 --- a/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs +++ b/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs @@ -100,15 +100,6 @@ conversionType is not MappingConversionType.EnumToString and not MappingConversi public INewInstanceMapping? FindNewInstanceMapping(IMethodSymbol method) { - // // check for recursion loop returning null to prevent a loop or default when recursion limit is reached. - // var count = _parentTypes.GetDepth(mappingKey); - // if (count >= 1) - // { - // return count >= Configuration.Properties.MaxRecursionDepth + 2 - // ? new DefaultMemberMapping(mappingKey.Source, mappingKey.Target) - // : null; - // } - INewInstanceMapping? mapping = InlinedMappings.FindNewInstanceUserMapping(method, out var isInlined); if (mapping == null) return null; diff --git a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs index e833374829..49bc58cff3 100644 --- a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionTest.cs @@ -116,22 +116,6 @@ public Task RecordToRecordManualListMapping() return TestHelper.VerifyGenerator(source); } - // [Fact] - // public Task ClassToClassWithUserImplemented() - // { - // var source = TestSourceBuilder.MapperWithBodyAndTypes( - // """ - // private partial System.Linq.IQueryable Map(System.Linq.IQueryable source); - // - // private D MapToD(C v) => new D { Value = v.Value + "-mapped" }; - // """, - // "class A { public string StringValue { get; set; } public C NestedValue { get; set; } }", - // "class B { public string StringValue { get; set; } public D NestedValue { get; set; } }", - // "class C { public string Value { get; set; } }", - // "class D { public string Value { get; set; } }" - // ); - // } - [Fact] public Task CtorShouldSkipUnmatchedOptionalParameters() { From 60282848a314e32640df22c6f9758884eeb8a3d4 Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Sat, 30 Mar 2024 13:00:45 +0000 Subject: [PATCH 6/6] chore: update public api --- src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt b/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt index 2898b86424..b758f42277 100644 --- a/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt +++ b/src/Riok.Mapperly.Abstractions/PublicAPI.Shipped.txt @@ -155,7 +155,7 @@ Riok.Mapperly.Abstractions.UserMappingAttribute.Default.get -> bool Riok.Mapperly.Abstractions.UserMappingAttribute.Default.set -> void Riok.Mapperly.Abstractions.MapPropertyAttribute.Use.get -> string? Riok.Mapperly.Abstractions.MapPropertyAttribute.Use.set -> void -Riok.Mapperly.Abstractions.MapperAttribute.MaxRecursionDepth.get -> int +Riok.Mapperly.Abstractions.MapperAttribute.MaxRecursionDepth.get -> uint Riok.Mapperly.Abstractions.MapperAttribute.MaxRecursionDepth.set -> void Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute Riok.Mapperly.Abstractions.MapperMaxRecursionDepthAttribute.MapperMaxRecursionDepthAttribute(uint maxRecursionDepth) -> void