diff --git a/README.md b/README.md index 5a67fef..c646381 100644 --- a/README.md +++ b/README.md @@ -793,6 +793,12 @@ Numeric widening, enum into a string, enum into a different enum by name, `DateT `DateTimeOffset`, nullable lifting, nested objects, `List` and array collections, and flattened paths up to four levels deep. +A generated mapper is planned from the declared types. Where the source, a member or a collection +element is declared as a type that can be derived from and holds an instance of a derived type, that +one instance goes to the engine, which maps by the runtime type as it does without the generator. A +sealed type has no such check. Under NativeAOT the engine cannot build a pair at run time, so declare +the derived pair as well, or the call throws `NotSupportedException` naming it. + These are refused on purpose, and each is a refusal rather than a gap. A refused pair reports `MSG001`, keeps mapping through the engine, and the call site does not change: diff --git a/changelog.d/sourcegen-runtime-types.fixed.md b/changelog.d/sourcegen-runtime-types.fixed.md new file mode 100644 index 0000000..45cd238 --- /dev/null +++ b/changelog.d/sourcegen-runtime-types.fixed.md @@ -0,0 +1,13 @@ +- **A generated mapper now maps a derived instance the way the engine does.** A member declared + `Animal` and holding a `Dog`, a `List` or `Animal[]` element holding one, and a `Dog` behind + a variable typed `Animal` were all mapped as `Animal`, so `Breed` came back empty where the engine + fills it. Generated code now checks the runtime type and hands anything that is not exactly the + declared type to the engine. A sealed type gets no check. Under NativeAOT, declare the derived pair + too, or the call throws `NotSupportedException` instead of returning the base members only. +- **An array into a `List` of the same class no longer shares the elements.** `Tag[]` into + `List` put the source instances in the list, where the engine builds a new `Tag` for each. A + pair whose element class cannot be generated is now refused with `MSG001` and maps through the + engine. +- **A collection whose destination element is an interface is refused.** An `IThing[]` into a + `List` put the source instances in the list, where the engine leaves each element null. + The pair now gets `MSG001` and maps through the engine. diff --git a/src/Mapsicle.SourceGen/MapperGenerator.cs b/src/Mapsicle.SourceGen/MapperGenerator.cs index 766e5bb..4dc7089 100644 --- a/src/Mapsicle.SourceGen/MapperGenerator.cs +++ b/src/Mapsicle.SourceGen/MapperGenerator.cs @@ -645,6 +645,7 @@ private static bool HasAttribute(ISymbol symbol, string fullName) => } var helper = context.Begin(key, HelperKind.Object, Full(source), Full(destination)); + helper.SourceCanBeDerived = CanBeDerived(source); var assignments = PlanMembers(source, destination, context, out _); context.End(key); @@ -722,19 +723,23 @@ private static bool HasAttribute(ISymbol symbol, string fullName) => /// private static string? ElementConvert(ITypeSymbol from, ITypeSymbol to, string expression, PlanContext context) { - // Identical element type passes straight through, which is what the engine does: a - // List into a List hands back the same Tag instances on both lanes. - if (SymbolEqualityComparer.Default.Equals(from, to)) return expression; - - // A reference-assignable element is NOT passed through, and that is the difference that - // matters. A List into a List handed back the same Dog instances on the - // generated lane, so mutating the DTO mutated the entity and the runtime type leaked - // through a contract that said Animal. The engine builds a fresh Animal, so this maps. + // The engine maps each element into the destination element type, cannot construct an + // interface, and leaves the element null. Passing the source instance through gave an + // IThing[] into a List its elements on the generated lane and nulls on the other. + if (to.TypeKind == TypeKind.Interface) return null; + + // A mappable element is never passed through, identical type included. A List into + // a List handed back the same Dog instances on the generated lane, and so did a + // Tag[] into a List, so mutating the DTO mutated the entity. The engine builds a + // fresh element for both. A List into a List never gets here: the member is + // assignable as it stands and both lanes hand over the same list. if (IsMappable(from) && to.TypeKind == TypeKind.Class && !IsString(to)) { return NestedCall(from, to, expression, context); } + if (SymbolEqualityComparer.Default.Equals(from, to)) return expression; + if (IsReferenceAssignable(from, to)) return expression; if (from.TypeKind == TypeKind.Enum && to.TypeKind == TypeKind.Enum @@ -1157,6 +1162,10 @@ private static bool IsEnumerable(ITypeSymbol type) => private static bool IsMappable(ITypeSymbol type) => type.TypeKind is TypeKind.Class or TypeKind.Interface && !IsString(type) && !IsEnumerable(type); + /// Whether a variable of this type can hold an instance of some other type. + private static bool CanBeDerived(ITypeSymbol type) => + type.TypeKind == TypeKind.Interface || (type.TypeKind == TypeKind.Class && !type.IsSealed); + private static bool IsReferenceAssignable(ITypeSymbol from, ITypeSymbol to) { if (to.TypeKind == TypeKind.Interface) @@ -1298,6 +1307,9 @@ internal Helper(string name, HelperKind kind, string sourceType, string destinat internal string? ElementSourceType { get; set; } internal string? ElementDestinationType { get; set; } internal bool SourceIsArray { get; set; } + + /// Set when the source is a type an instance of a derived type can stand in for. + internal bool SourceCanBeDerived { get; set; } } /// @@ -1451,12 +1463,21 @@ private static string Emit(List plans) private static void EmitBody( StringBuilder sb, string name, string sourceType, string destType, - List assignments, bool nullable) + List assignments, bool nullable, bool guardRuntimeType = false) { var question = nullable ? "?" : ""; sb.AppendLine($" internal static {destType}{question} {name}({sourceType}{question} source)"); sb.AppendLine(" {"); if (nullable) sb.AppendLine(" if (source is null) return null;"); + + // The members below were planned from the declared type. A member declared Animal and + // holding a Dog came out with Breed empty, where the engine maps by the runtime type and + // fills it. Anything that is not exactly the declared type goes to the engine. + if (guardRuntimeType) + { + sb.AppendLine($" if (source.GetType() != typeof({sourceType})) return global::Mapsicle.Mapper.MapTo<{destType}>((object)source);"); + } + sb.AppendLine($" return new {destType}"); sb.AppendLine(" {"); @@ -1491,7 +1512,9 @@ private static void EmitHelper(StringBuilder sb, Helper helper) if (helper.Kind == HelperKind.Object) { - EmitBody(sb, helper.Name, helper.SourceType, helper.DestinationType, helper.Assignments, nullable: true); + EmitBody( + sb, helper.Name, helper.SourceType, helper.DestinationType, helper.Assignments, + nullable: true, guardRuntimeType: helper.SourceCanBeDerived); return; } @@ -1615,6 +1638,15 @@ private static void EmitGroup(StringBuilder sb, IEnumerable plans, stri sb.AppendLine($"{indent} public static TDest? MapTo(this {src}{(isValueType ? "" : "?")} source)"); sb.AppendLine($"{indent} {{"); if (!isValueType) sb.AppendLine($"{indent} if (source is null) return default;"); + + // A derived instance behind a variable of the declared type bound here and was + // mapped as the declared type, so a member only the derived type has stayed at its + // default. The same call without the generator maps by the runtime type. + if (CanBeDerived(group.First().Source)) + { + sb.AppendLine($"{indent} if (source.GetType() != typeof({src})) return global::Mapsicle.Mapper.MapTo((object)source);"); + } + sb.AppendLine(); foreach (var plan in group) diff --git a/tests/Mapsicle.SourceGen.Tests/LaneConformanceTests.cs b/tests/Mapsicle.SourceGen.Tests/LaneConformanceTests.cs index 25b7e62..b3f1c0a 100644 --- a/tests/Mapsicle.SourceGen.Tests/LaneConformanceTests.cs +++ b/tests/Mapsicle.SourceGen.Tests/LaneConformanceTests.cs @@ -25,6 +25,13 @@ [assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.ConfCovariant), typeof(Mapsicle.SourceGen.Tests.ConfCovariantDto))] [assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.ConfShielded), typeof(Mapsicle.SourceGen.Tests.ConfShieldedDto))] [assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.ConfEnumFit), typeof(Mapsicle.SourceGen.Tests.ConfEnumFitDto))] +[assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.ConfOwner), typeof(Mapsicle.SourceGen.Tests.ConfOwnerDto))] +[assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.ConfKennel), typeof(Mapsicle.SourceGen.Tests.ConfKennelDto))] +[assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.ConfTagged), typeof(Mapsicle.SourceGen.Tests.ConfTaggedDto))] +[assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.FaceArray), typeof(Mapsicle.SourceGen.Tests.FaceArrayDto))] +[assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.FaceImplArray), typeof(Mapsicle.SourceGen.Tests.FaceImplArrayDto))] +[assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.ConfRoot), typeof(Mapsicle.SourceGen.Tests.ConfRootDto))] +[assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.ConfLoopOwner), typeof(Mapsicle.SourceGen.Tests.ConfLoopOwnerDto))] [assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.LossyEnum), typeof(Mapsicle.SourceGen.Tests.LossyEnumDto))] [assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.LossyFloat), typeof(Mapsicle.SourceGen.Tests.LossyFloatDto))] [assembly: MapsicleGenerate(typeof(Mapsicle.SourceGen.Tests.LossyDouble), typeof(Mapsicle.SourceGen.Tests.LossyDoubleDto))] @@ -184,6 +191,46 @@ public class ConfShieldedDto [IgnoreMap] public bool IsAdmin { get; set; } } + // A member or an element declared as the base and holding a derived instance. The engine maps + // by the runtime type, so the derived members arrive. The emitter planned the declared type and + // left them at their defaults. + public class ConfPet { public string Name { get; set; } = ""; } + public class ConfCollie : ConfPet { public string Breed { get; set; } = ""; } + public class ConfPetDto { public string Name { get; set; } = ""; public string Breed { get; set; } = ""; } + public class ConfOwner { public ConfPet? Pet { get; set; } } + public class ConfOwnerDto { public ConfPetDto? Pet { get; set; } } + public class ConfKennel { public List Pets { get; set; } = new(); public ConfPet[] Boarders { get; set; } = System.Array.Empty(); } + public class ConfKennelDto { public List Pets { get; set; } = new(); public List Boarders { get; set; } = new(); } + + // An array into a list of the same element type. The engine builds a new element for each one, + // and the emitter put the source instances in the list. + public class ConfTag { public string Label { get; set; } = ""; } + public class ConfTagged { public ConfTag[] Tags { get; set; } = System.Array.Empty(); } + public class ConfTaggedDto { public List Tags { get; set; } = new(); } + + // Elements typed as an interface. The engine cannot construct one and leaves each element + // null, where the emitter put the source instance in the list. Named outside the Conf prefix + // because the pairs are refused, as the Lossy ones are. + public interface IFaceNamed { string Label { get; set; } } + public class FaceNamed : IFaceNamed { public string Label { get; set; } = ""; } + public class FaceArray { public IFaceNamed[] Items { get; set; } = System.Array.Empty(); } + public class FaceArrayDto { public List Items { get; set; } = new(); } + public class FaceImplArray { public FaceNamed[] Items { get; set; } = System.Array.Empty(); } + public class FaceImplArrayDto { public List Items { get; set; } = new(); } + + // A cycle the declared types do not have and a derived type adds. The planner sees no cycle, + // so the pair is generated, and the data can still loop. + public class ConfLoopPet { public string Name { get; set; } = ""; } + public class ConfLoopBack : ConfLoopPet { public ConfLoopOwner? Owner { get; set; } } + public class ConfLoopOwner { public ConfLoopPet? Pet { get; set; } } + public class ConfLoopPetDto { public string Name { get; set; } = ""; public ConfLoopOwnerDto? Owner { get; set; } } + public class ConfLoopOwnerDto { public ConfLoopPetDto? Pet { get; set; } } + + // The declared pair itself, reached through a variable typed as the base. + public class ConfRoot { public string Name { get; set; } = ""; } + public class ConfRootSpecial : ConfRoot { public string Extra { get; set; } = ""; } + public class ConfRootDto { public string Name { get; set; } = ""; public string Extra { get; set; } = ""; } + /// /// One table of cases, run through the runtime lane and the generated lane, asserting they agree. /// @@ -509,6 +556,126 @@ public void ACollectionOfAssignableElementsCopiesRatherThanAliases() Assert.Equal(interpreted.Pets[0].Name, generated.Pets[0].Name); } + [Fact] + public void ANestedMemberHoldingADerivedInstanceAgrees() => + LanesAgree( + new ConfOwner { Pet = new ConfCollie { Name = "Rex", Breed = "collie" } }, + d => d.Pet!.Name, d => d.Pet!.Breed); + + [Fact] + public void ANestedMemberHoldingTheDeclaredTypeAgrees() => + LanesAgree( + new ConfOwner { Pet = new ConfPet { Name = "Rex" } }, + d => d.Pet!.Name, d => d.Pet!.Breed); + + [Fact] + public void ANullNestedMemberOfAnOpenTypeAgrees() => + LanesAgree(new ConfOwner { Pet = null }, d => d.Pet); + + [Fact] + public void CollectionElementsHoldingDerivedInstancesAgree() => + LanesAgree( + new ConfKennel + { + Pets = { new ConfPet { Name = "Tom" }, new ConfCollie { Name = "Rex", Breed = "collie" } }, + Boarders = new ConfPet[] { new ConfCollie { Name = "Fly", Breed = "border" }, new ConfPet { Name = "Sam" } }, + }, + d => d.Pets.Count, d => d.Pets[0].Name, d => d.Pets[0].Breed, d => d.Pets[1].Name, d => d.Pets[1].Breed, + d => d.Boarders.Count, d => d.Boarders[0].Name, d => d.Boarders[0].Breed, d => d.Boarders[1].Breed); + + [Fact] + public void AnArrayIntoAListOfTheSameElementCopiesInBothLanes() + { + var source = new ConfTagged { Tags = new[] { new ConfTag { Label = "a" }, new ConfTag { Label = "b" } } }; + + var generated = ((object)source).MapTo(); + + using var runtime = MapperFactory.Create(); + var interpreted = runtime.MapTo(source); + + Assert.False(ReferenceEquals(interpreted!.Tags[0], source.Tags[0]), "the engine aliased the source element"); + Assert.False(ReferenceEquals(generated!.Tags[0], source.Tags[0]), "the generated lane aliased the source element"); + Assert.Equal(new[] { "a", "b" }, generated.Tags.Select(t => t.Label).ToArray()); + Assert.Equal(new[] { "a", "b" }, interpreted.Tags.Select(t => t.Label).ToArray()); + } + + [Fact] + public void InterfaceElementPairsAreRefusedAndMapAsTheEngineDoes() + { + var registry = typeof(Mapper) + .GetField("_generatedPairs", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static) + !.GetValue(null)!; + var keys = ((System.Collections.IEnumerable)registry.GetType().GetProperty("Keys")!.GetValue(registry)!) + .Cast() + .Select(k => k.ToString() ?? "") + .ToArray(); + + foreach (var name in new[] { nameof(FaceArray), nameof(FaceImplArray) }) + { + Assert.DoesNotContain(keys, k => k.Contains(name + ",", StringComparison.Ordinal) || k.Contains(name + ")", StringComparison.Ordinal)); + } + + var one = new FaceNamed { Label = "a" }; + using var runtime = MapperFactory.Create(); + + var faces = new FaceArray { Items = new IFaceNamed[] { one } }; + Assert.Equal(new IFaceNamed?[] { null }, runtime.MapTo(faces)!.Items); + Assert.Equal(new IFaceNamed?[] { null }, ((object)faces).MapTo()!.Items); + + var impls = new FaceImplArray { Items = new[] { one } }; + Assert.Equal(new IFaceNamed?[] { null }, runtime.MapTo(impls)!.Items); + Assert.Equal(new IFaceNamed?[] { null }, ((object)impls).MapTo()!.Items); + } + + [Fact] + public void ADerivedInstanceBehindTheDeclaredTypeAgrees() + { + ConfRoot source = new ConfRootSpecial { Name = "n", Extra = "x" }; + + // Bound at compile time to the generated extension for ConfRoot, which is the door the + // declared type opens. The registry door is keyed by runtime type and never sees this. + var generated = source.MapTo(); + + using var runtime = MapperFactory.Create(); + var interpreted = runtime.MapTo(source); + + Assert.Equal("x", interpreted!.Extra); + Assert.Equal("n", generated!.Name); + Assert.Equal(interpreted.Extra, generated.Extra); + } + + [Fact] + public void ACycleThroughADerivedInstanceEndsInBothLanes() + { + var owner = new ConfLoopOwner(); + owner.Pet = new ConfLoopBack { Name = "Rex", Owner = owner }; + + var generated = ((object)owner).MapTo(); + + using var runtime = MapperFactory.Create(); + var interpreted = runtime.MapTo(owner); + + Assert.Equal(Depth(interpreted), Depth(generated)); + Assert.Equal("Rex", generated!.Pet!.Name); + Assert.Equal("Rex", generated.Pet.Owner!.Pet!.Name); + + static int Depth(ConfLoopOwnerDto? dto) + { + var depth = 0; + for (var current = dto; current is not null; current = current.Pet?.Owner) depth++; + return depth; + } + } + + [Fact] + public void TheDeclaredTypeItselfStillMapsThroughTheExtension() + { + var generated = new ConfRoot { Name = "n" }.MapTo(); + + Assert.Equal("n", generated!.Name); + Assert.Equal("", generated.Extra); + } + // ---- refusals --------------------------------------------------------------------------- public class ConfInner { public string City { get; set; } = ""; } @@ -575,8 +742,9 @@ public void TheTableCoversEveryPairTheAssemblyDeclares() { nameof(ConfCaseEnum), nameof(ConfCasing), nameof(ConfControlled), nameof(ConfCovariant), nameof(ConfCrossEnum), nameof(ConfDerived), nameof(ConfEnumFit), nameof(ConfEnumText), nameof(ConfFlat), nameof(ConfFlatten), - nameof(ConfKinds), nameof(ConfLift), nameof(ConfList), nameof(ConfNest), - nameof(ConfNullable), nameof(ConfPartial), nameof(ConfShielded), nameof(ConfStamp), nameof(ConfWiden), + nameof(ConfKennel), nameof(ConfKinds), nameof(ConfLift), nameof(ConfList), nameof(ConfLoopOwner), nameof(ConfNest), + nameof(ConfNullable), nameof(ConfOwner), nameof(ConfPartial), nameof(ConfRoot), nameof(ConfShielded), nameof(ConfStamp), + nameof(ConfTagged), nameof(ConfWiden), }; Assert.Equal(covered, declared);