From 22899be49678fafaa5bc45930f23dd6c1c042ee8 Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Mon, 17 Jul 2023 22:37:23 +0100 Subject: [PATCH] feat: add error handling --- .../NewValueTupleMappingBodyBuilder.cs | 4 + ...NewInstanceObjectPropertyMappingBuilder.cs | 10 +- .../NewValueTupleConstructorMapping.cs | 19 +- .../NewValueTupleExpressionMapping.cs | 20 +- src/Riok.Mapperly/Emit/SyntaxFactoryHelper.cs | 3 + .../Mapping/ValueTupleTest.cs | 209 +++++++++++++----- ...yShouldMapNestedTuple#Mapper.g.verified.cs | 12 + ...ShouldMapToTupleField#Mapper.g.verified.cs | 19 ++ ...opertyToTupleProperty#Mapper.g.verified.cs | 12 + 9 files changed, 233 insertions(+), 75 deletions(-) create mode 100644 test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.MapPropertyShouldMapNestedTuple#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.MapPropertyShouldMapToTupleField#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.TuplePropertyToTupleProperty#Mapper.g.verified.cs diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs index a7023cd054..9febdda929 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs @@ -66,6 +66,10 @@ out HashSet mappedTargetMemberNames foreach (var targetMember in namedTargetType.TupleElements) { + if (!ctx.TargetMembers.ContainsKey(targetMember.Name)) + { + return false; + } if (!TryFindConstructorParameterSourcePath(ctx, targetMember, out var sourcePath)) { ctx.BuilderContext.ReportDiagnostic( diff --git a/src/Riok.Mapperly/Descriptors/MappingBuilders/NewInstanceObjectPropertyMappingBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBuilders/NewInstanceObjectPropertyMappingBuilder.cs index 0f32d9e2a1..005366f0b5 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBuilders/NewInstanceObjectPropertyMappingBuilder.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBuilders/NewInstanceObjectPropertyMappingBuilder.cs @@ -30,9 +30,13 @@ public static class NewInstanceObjectPropertyMappingBuilder if (ctx.IsConversionEnabled(MappingConversionType.Enumerable) && ctx.Target.IsTupleType) { // inline expressions don't support tuple expressions so ValueTuple is used instead - return ctx.IsExpression - ? new NewValueTupleConstructorMapping(ctx.Source, ctx.Target, ctx.Types.Get()) - : new NewValueTupleExpressionMapping(ctx.Source, ctx.Target); + if (ctx.IsExpression) + { + return new NewValueTupleConstructorMapping(ctx.Source, ctx.Target); + } + + var expectedArgumentCount = (ctx.Target as INamedTypeSymbol)!.TupleElements.Length; + return new NewValueTupleExpressionMapping(ctx.Source, ctx.Target, expectedArgumentCount); } // inline expressions don't support method property mappings diff --git a/src/Riok.Mapperly/Descriptors/Mappings/NewValueTupleConstructorMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/NewValueTupleConstructorMapping.cs index 7bae120eec..4c71bd8cf8 100644 --- a/src/Riok.Mapperly/Descriptors/Mappings/NewValueTupleConstructorMapping.cs +++ b/src/Riok.Mapperly/Descriptors/Mappings/NewValueTupleConstructorMapping.cs @@ -2,7 +2,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Riok.Mapperly.Descriptors.Mappings.MemberMappings; -using Riok.Mapperly.Emit; +using static Riok.Mapperly.Emit.SyntaxFactoryHelper; namespace Riok.Mapperly.Descriptors.Mappings; @@ -13,14 +13,11 @@ namespace Riok.Mapperly.Descriptors.Mappings; /// public class NewValueTupleConstructorMapping : TypeMapping, INewValueTupleMapping { + private const string ValueTupleName = "global::System.ValueTuple"; private readonly HashSet _constructorPropertyMappings = new(); - private readonly ITypeSymbol _valueTupleSymbol; - public NewValueTupleConstructorMapping(ITypeSymbol sourceType, ITypeSymbol targetType, ITypeSymbol valueTupleSymbol) - : base(sourceType, targetType) - { - _valueTupleSymbol = valueTupleSymbol; - } + public NewValueTupleConstructorMapping(ITypeSymbol sourceType, ITypeSymbol targetType) + : base(sourceType, targetType) { } public void AddConstructorParameterMapping(ValueTupleConstructorParameterMapping mapping) => _constructorPropertyMappings.Add(mapping); @@ -28,11 +25,9 @@ public override ExpressionSyntax Build(TypeMappingBuildContext ctx) { // new ValueTuple(ctorArgs) var ctorArgs = _constructorPropertyMappings.Select(x => x.BuildArgument(ctx, insideExpression: true)).ToArray(); - var genericName = SyntaxFactory.GenericName(SyntaxFactoryHelper.FullyQualifiedIdentifierName(_valueTupleSymbol)); - var typeArguments = SyntaxFactoryHelper.TypeArgumentList( - (TargetType as INamedTypeSymbol)!.TypeArguments.Select(SyntaxFactoryHelper.NonNullableIdentifier).ToArray() - ); + var genericName = SyntaxFactory.GenericName(ValueTupleName); + var typeArguments = TypeArgumentList((TargetType as INamedTypeSymbol)!.TypeArguments.Select(NonNullableIdentifier)); var typedValue = genericName.WithTypeArgumentList(typeArguments); - return SyntaxFactory.ObjectCreationExpression(typedValue).WithArgumentList(SyntaxFactoryHelper.ArgumentList(ctorArgs)); + return SyntaxFactory.ObjectCreationExpression(typedValue).WithArgumentList(ArgumentList(ctorArgs)); } } diff --git a/src/Riok.Mapperly/Descriptors/Mappings/NewValueTupleExpressionMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/NewValueTupleExpressionMapping.cs index 0304f7ffb5..0847a1b4dc 100644 --- a/src/Riok.Mapperly/Descriptors/Mappings/NewValueTupleExpressionMapping.cs +++ b/src/Riok.Mapperly/Descriptors/Mappings/NewValueTupleExpressionMapping.cs @@ -14,11 +14,16 @@ namespace Riok.Mapperly.Descriptors.Mappings; /// public class NewValueTupleExpressionMapping : ObjectMemberMethodMapping, INewValueTupleMapping { + private readonly int _argumentCount; + private const string NoMappingComment = "// Could not generate mapping"; private const string TargetVariableName = "target"; private readonly HashSet _constructorPropertyMappings = new(); - public NewValueTupleExpressionMapping(ITypeSymbol sourceType, ITypeSymbol targetType) - : base(sourceType, targetType) { } + public NewValueTupleExpressionMapping(ITypeSymbol sourceType, ITypeSymbol targetType, int argumentCount) + : base(sourceType, targetType) + { + _argumentCount = argumentCount; + } public void AddConstructorParameterMapping(ValueTupleConstructorParameterMapping mapping) => _constructorPropertyMappings.Add(mapping); @@ -29,6 +34,11 @@ public NewValueTupleExpressionMapping(ITypeSymbol sourceType, ITypeSymbol target public override ExpressionSyntax Build(TypeMappingBuildContext ctx) { + if (_constructorPropertyMappings.Count != _argumentCount) + { + return ThrowNotImplementedException().WithLeadingTrivia(TriviaList(Comment(NoMappingComment))); + } + if (HasMemberMappings()) { return base.Build(ctx); @@ -40,6 +50,12 @@ public override ExpressionSyntax Build(TypeMappingBuildContext ctx) public override IEnumerable BuildBody(TypeMappingBuildContext ctx) { + if (_constructorPropertyMappings.Count != _argumentCount) + { + yield return ExpressionStatement(ThrowNotImplementedException()).WithLeadingTrivia(TriviaList(Comment(NoMappingComment))); + yield break; + } + // (Name:.. ,..); var ctorArgs = _constructorPropertyMappings.Select(x => x.BuildArgument(ctx, insideExpression: false)).ToArray(); var tupleCreationExpression = TupleExpression(SeparatedList(ctorArgs)); diff --git a/src/Riok.Mapperly/Emit/SyntaxFactoryHelper.cs b/src/Riok.Mapperly/Emit/SyntaxFactoryHelper.cs index 14c74f4f61..834876b8dc 100644 --- a/src/Riok.Mapperly/Emit/SyntaxFactoryHelper.cs +++ b/src/Riok.Mapperly/Emit/SyntaxFactoryHelper.cs @@ -396,6 +396,9 @@ public static ArgumentListSyntax ArgumentList(params ExpressionSyntax[] argSynta public static TypeArgumentListSyntax TypeArgumentList(params TypeSyntax[] argSyntaxes) => SyntaxFactory.TypeArgumentList(CommaSeparatedList(argSyntaxes)); + public static TypeArgumentListSyntax TypeArgumentList(IEnumerable argSyntaxes) => + SyntaxFactory.TypeArgumentList(CommaSeparatedList(argSyntaxes)); + public static ArgumentListSyntax ArgumentList(params ArgumentSyntax[] args) => SyntaxFactory.ArgumentList(CommaSeparatedList(args)); public static SeparatedSyntaxList CommaSeparatedList(IEnumerable nodes, bool insertTrailingComma = false) diff --git a/test/Riok.Mapperly.Tests/Mapping/ValueTupleTest.cs b/test/Riok.Mapperly.Tests/Mapping/ValueTupleTest.cs index 37a2c4bceb..032c1c22f2 100644 --- a/test/Riok.Mapperly.Tests/Mapping/ValueTupleTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/ValueTupleTest.cs @@ -157,89 +157,182 @@ public void ClassToTuple() } [Fact] - public void NestedClassToTuple() + public void TupleToTupleWithIgnoredSource() { - var source = TestSourceBuilder.Mapping( - "A", - "(int NestedValue, string C)", - "public class A { public B Nested { get;set;} public int C {get;set;} }", - "public class B { public int Value { get;set;} }" + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + [MapperIgnoreSource("C")] + partial (int, string) Map((int A, string B, int C) source); + """ ); - TestHelper - .GenerateMapper(source) - .Should() - .HaveSingleMethodBody("return (NestedValue: source.Nested.Value, C: source.C.ToString());"); + TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return (source.A, source.B);"); } [Fact] - public void TupleToTupleWithUnmappedSourceShouldDiagnostic() + public void TupleToTupleWithIgnoredSourceByPosition() { var source = TestSourceBuilder.MapperWithBodyAndTypes( """ -[MapperIgnoreSource("Item3")] -partial (int, string) Map((int A, string B, int C) source); -""" + [MapperIgnoreSource("Item3")] + partial (int, string) Map((int A, string B, int) source); + """ ); TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return (source.A, source.B);"); } [Fact] - public void TupleToTupleWithIgnoredSource() + public void ClassToTupleWithIgnoredSource() { var source = TestSourceBuilder.MapperWithBodyAndTypes( """ -[MapperIgnoreSource("C")] -partial (int, string) Map((int A, string B, int C) source); -""" + [MapperIgnoreSource("A")] + partial (int, int) Map(B source); + """, + "public class B { public int Item1 { get;set;} public int A {get;set;} public int Item2 {get;set;} }" ); - TestHelper.GenerateMapper(source, TestHelperOptions.AllowDiagnostics).Should().HaveSingleMethodBody("return (source.A, source.B);"); + TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return (source.Item1, source.Item2);"); } [Fact] - public void TupleToTupleWithIgnoredSourceByPosition() + public void IgnoredNamedSourceWithPositionalShouldDiagnostic() { var source = TestSourceBuilder.MapperWithBodyAndTypes( """ -[MapperIgnoreSource("Item3")] -partial (int, string) Map((int A, string B, int) source); -""" + [MapperIgnoreSource("Item3")] + partial (int, string) Map((int A, string B, int C) source); + """ ); - TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return (source.A, source.B);"); + TestHelper + .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .Should() + .HaveDiagnostic(DiagnosticDescriptors.IgnoredSourceMemberNotFound) + .HaveSingleMethodBody("return (source.A, source.B);"); } [Fact] - public void TupleToTupleWithIgnoredSourceShouldDiagnostic() + public void TupleWithNonExistentIgnoreSourceShouldDiagnostic() { var source = TestSourceBuilder.MapperWithBodyAndTypes( """ -[MapperIgnoreSource("A")] -partial (int, int) Map((int, int A) source); -""" + [MapperIgnoreSource("D")] + partial (int, string) Map((int A, string B) source); + """ + ); + + TestHelper + .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .Should() + .HaveDiagnostic(DiagnosticDescriptors.IgnoredSourceMemberNotFound) + .HaveSingleMethodBody("return (source.A, source.B);"); + } + + [Fact] + public void InvalidTupleShouldDiagnostic() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + [MapperIgnoreSource("A")] + partial (int, int) Map((int, int A) source); + """ ); TestHelper .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) .Should() .HaveDiagnostic(DiagnosticDescriptors.SourceMemberNotFound) - .HaveSingleMethodBody("return ();"); + .HaveDiagnostic(DiagnosticDescriptors.NoConstructorFound) + .HaveSingleMethodBody( + """ + // Could not generate mapping + throw new System.NotImplementedException(); + """ + ); } [Fact] - public void ClassToTupleWithIgnoredSource() + public void IgnoreTargetTuple() { var source = TestSourceBuilder.MapperWithBodyAndTypes( """ -[MapperIgnoreSource("A")] -partial (int, int) Map(B source); -""", - "public class B { public int Item1 { get;set;} public int A {get;set;} public int Item2 {get;set;} }" + [MapperIgnoreTarget("A")] + partial (int, int A) Map((string, int A) source); + """ ); - TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return (source.Item1, source.Item2);"); + TestHelper + .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .Should() + .HaveDiagnostic(DiagnosticDescriptors.NoConstructorFound) + .HaveDiagnostic(DiagnosticDescriptors.SourceMemberNotMapped) + .HaveAssertedAllDiagnostics() + .HaveSingleMethodBody( + """ + // Could not generate mapping + throw new System.NotImplementedException(); + """ + ); + } + + [Fact] + public void IgnoreTargetByPosition() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + [MapperIgnoreTarget("Item1")] + partial (int, int A) Map((string, int A) source); + """ + ); + + TestHelper + .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .Should() + .HaveDiagnostic(DiagnosticDescriptors.NoConstructorFound) + .HaveDiagnostic(DiagnosticDescriptors.SourceMemberNotMapped) + .HaveAssertedAllDiagnostics() + .HaveSingleMethodBody( + """ + // Could not generate mapping + throw new System.NotImplementedException(); + """ + ); + } + + [Fact] + public void IgnoreTargetWithNonExistentTargetShouldDiagnostic() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + [MapperIgnoreTarget("B")] + partial (int, int A) Map((string, int) source); + """ + ); + + TestHelper + .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .Should() + .HaveDiagnostic(DiagnosticDescriptors.IgnoredTargetMemberNotFound) + .HaveSingleMethodBody("return (int.Parse(source.Item1), A: source.Item2);"); + } + + [Fact] + public void IgnoreTargetWithNonExistentPositionalShouldDiagnostic() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + [MapperIgnoreTarget("Item3")] + partial (int, int A) Map((string, int) source); + """ + ); + + TestHelper + .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .Should() + .HaveDiagnostic(DiagnosticDescriptors.IgnoredTargetMemberNotFound) + .HaveSingleMethodBody("return (int.Parse(source.Item1), A: source.Item2);"); } [Fact] @@ -247,17 +340,21 @@ public void TupleToTupleWithMapProperty() { var source = TestSourceBuilder.MapperWithBodyAndTypes( """ -[MapProperty("C", "A")] -[MapProperty("Item3", "Item2")] -partial (int A, int) Map((int B, int C, int) source); -""" + [MapProperty("C", "A")] + [MapProperty("Item3", "Item2")] + partial (int A, int) Map((int B, int C, int) source); + """ ); - TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return (A: source.C, source.Item3);"); + TestHelper + .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .Should() + .HaveDiagnostic(DiagnosticDescriptors.SourceMemberNotMapped) + .HaveSingleMethodBody("return (A: source.C, source.Item3);"); } [Fact] - public void TuplePropertyToTupleProperty() + public Task TuplePropertyToTupleProperty() { var source = TestSourceBuilder.Mapping( "A", @@ -266,26 +363,17 @@ public void TuplePropertyToTupleProperty() "class B { public (string A, int) Value { get; set; } }" ); - TestHelper - .GenerateMapper(source) - .Should() - .HaveSingleMethodBody( - """ - var target = new global::B(); - target.Value = (A: source.Value.A.ToString(), source.Value.Item2); - return target; - """ - ); + return TestHelper.VerifyGenerator(source); } [Fact] - public Task TuplePropertyToTupleProperty1() + public Task MapPropertyShouldMapToTupleField() { var source = TestSourceBuilder.MapperWithBodyAndTypes( """ - [MapProperty("Item2", "Item1.Value")] - partial (A, int) Map((B, string) source); - """, +[MapProperty("Item2", "Item1.Value")] +partial (A, int) Map((B, string) source); +""", "class A { public int Value { get; set; } }", "class B { public int Value { get; set; } }" ); @@ -294,7 +382,7 @@ public Task TuplePropertyToTupleProperty1() } [Fact] - public Task TuplePropertyToTupleProperty2() + public Task MapPropertyShouldMapNestedTuple() { var source = TestSourceBuilder.MapperWithBodyAndTypes( """ @@ -387,12 +475,17 @@ public void TupleToTupleWithMapPropertyWithImplicitNameShouldDiagnostic() [Fact] public void ClassToTupleWithNoMappingsShouldDiagnostic() { - var source = TestSourceBuilder.Mapping("A", "(int,int)", "public class A { }"); + var source = TestSourceBuilder.Mapping("A", "(int, int)", "public class A { }"); TestHelper .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) .Should() .HaveDiagnostic(DiagnosticDescriptors.SourceMemberNotFound) - .HaveSingleMethodBody("return ();"); + .HaveSingleMethodBody( + """ + // Could not generate mapping + throw new System.NotImplementedException(); + """ + ); } } diff --git a/test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.MapPropertyShouldMapNestedTuple#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.MapPropertyShouldMapNestedTuple#Mapper.g.verified.cs new file mode 100644 index 0000000000..d8432bf55c --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.MapPropertyShouldMapNestedTuple#Mapper.g.verified.cs @@ -0,0 +1,12 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + private partial ((int, int), int) Map(((int, int), string) source) + { + var target = (source.Item1, int.Parse(source.Item2)); + target.Item1.Item1 = int.Parse(source.Item2); + return target; + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.MapPropertyShouldMapToTupleField#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.MapPropertyShouldMapToTupleField#Mapper.g.verified.cs new file mode 100644 index 0000000000..ced7e60ab6 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.MapPropertyShouldMapToTupleField#Mapper.g.verified.cs @@ -0,0 +1,19 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + private partial (global::A, int) Map((global::B, string) source) + { + var target = (MapToA(source.Item1), int.Parse(source.Item2)); + target.Item1.Value = int.Parse(source.Item2); + return target; + } + + private global::A MapToA(global::B source) + { + var target = new global::A(); + target.Value = source.Value; + return target; + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.TuplePropertyToTupleProperty#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.TuplePropertyToTupleProperty#Mapper.g.verified.cs new file mode 100644 index 0000000000..c791967bf4 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/ValueTupleTest.TuplePropertyToTupleProperty#Mapper.g.verified.cs @@ -0,0 +1,12 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + private partial global::B Map(global::A source) + { + var target = new global::B(); + target.Value = (A: source.Value.A.ToString(), source.Value.Item2); + return target; + } +} \ No newline at end of file