From 05969a0af613fdc1db822affd921660eaab7054b Mon Sep 17 00:00:00 2001 From: Matt Morrissette Date: Sun, 12 Jun 2022 17:09:44 -0700 Subject: [PATCH 1/2] Add support for CSharpHelper for List literals Fixes #19274 Also relates to https://github.com/npgsql/efcore.pg/pull/2402 --- .../Design/Internal/CSharpHelper.cs | 99 +++++++++++++++++++ .../Design/Internal/CSharpHelperTest.cs | 45 +++++++++ 2 files changed, 144 insertions(+) diff --git a/src/EFCore.Design/Design/Internal/CSharpHelper.cs b/src/EFCore.Design/Design/Internal/CSharpHelper.cs index 9ee6e914bc6..c79d241af75 100644 --- a/src/EFCore.Design/Design/Internal/CSharpHelper.cs +++ b/src/EFCore.Design/Design/Internal/CSharpHelper.cs @@ -737,6 +737,71 @@ public virtual string Literal(object?[,] values) return builder.ToString(); } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual string Literal(IList values, bool vertical = false) + => List(typeof(T), values, vertical); + + private string List(Type type, IEnumerable values, bool vertical = false) + { + var builder = new IndentedStringBuilder(); + + builder.Append("new List<") + .Append(Reference(type)) + .Append("> {"); + + var first = true; + foreach (var value in values) + { + if (first) + { + if (vertical) + { + builder.AppendLine(); + builder.IncrementIndent(); + } + else + { + builder.Append(" "); + } + first = false; + } + else + { + builder.Append(","); + + if (vertical) + { + builder.AppendLine(); + } + else + { + builder.Append(" "); + } + } + + builder.Append(UnknownLiteral(value)); + } + + if (vertical) + { + builder.AppendLine(); + builder.DecrementIndent(); + } + else + { + builder.Append(" "); + } + + builder.Append("}"); + + return builder.ToString(); + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -844,6 +909,11 @@ public virtual string UnknownLiteral(object? value) return Array(literalType.GetElementType()!, array); } + if (value is IList list && value.GetType().IsGenericType && value.GetType().GetGenericTypeDefinition() == typeof(List<>)) + { + return List(value.GetType().GetGenericArguments()[0], list); + } + var mapping = _typeMappingSource.FindMapping(literalType); if (mapping != null) { @@ -878,6 +948,35 @@ private bool HandleExpression(Expression expression, StringBuilder builder, bool HandleList(((NewArrayExpression)expression).Expressions, builder, simple: true); + builder + .Append(" }"); + + return true; + case ExpressionType.ListInit: + var listExpr = (ListInitExpression)expression; + if (listExpr.Initializers.Any(i => i.Arguments.Count != 1)) + { + // If there is an initializer with more than one argument we can't make a literal cleanly + return false; + } + + builder + .Append("new ") + .Append(Reference(expression.Type)); + + if (listExpr.NewExpression.Arguments.Count > 0 && !HandleArguments(listExpr.NewExpression.Arguments, builder)) + { + return false; + } + + builder + .Append(" { "); + + if (!HandleList(listExpr.Initializers.Select(i => i.Arguments.First()), builder, simple: true)) + { + return false; + } + builder .Append(" }"); diff --git a/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs b/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs index d03c74c1aa9..be2ae861898 100644 --- a/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs +++ b/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs @@ -121,6 +121,24 @@ public void Literal_works_when_many_ByteArray() new byte[] { 1, 2 }, "new byte[] { 1, 2 }"); + [ConditionalFact] + public void Literal_works_when_empty_list() + => Literal_works( + new List(), + @"new List { }"); + + [ConditionalFact] + public void Literal_works_when_list_without_ctor_arguments() + => Literal_works( + new List { "one", "two" }, + @"new List { ""one"", ""two"" }"); + + [ConditionalFact] + public void Literal_works_when_list_with_ctor_arguments() + => Literal_works( + new List(new [] { "one" }) { "two", "three" }, + @"new List { ""one"", ""two"", ""three"" }"); + [ConditionalFact] public void Literal_works_when_multiline_string() => Literal_works( @@ -607,6 +625,33 @@ public void Literal_with_add() new CSharpHelper(typeMapping).UnknownLiteral(new SimpleTestType())); } + [ConditionalFact] + public void Literal_with_list_init_without_ctor_arguments() + { + var typeMapping = CreateTypeMappingSource( + v => Expression.ListInit( + Expression.New(typeof(List<>).MakeGenericType(typeof(string))), + Expression.Constant("one"), Expression.Constant("two"))); + + Assert.Equal( + @"new List { ""one"", ""two"" }", + new CSharpHelper(typeMapping).UnknownLiteral(new SimpleTestType())); + } + + [ConditionalFact] + public void Literal_with_list_init_with_ctor_arguments() + { + var constructor = typeof(List<>).MakeGenericType((typeof(string))).GetConstructor(new[] { typeof(IEnumerable) })!; + var typeMapping = CreateTypeMappingSource( + v => Expression.ListInit( + Expression.New(constructor, Expression.NewArrayInit(typeof(string), Expression.Constant("one"))), + Expression.Constant("one"), Expression.Constant("two"))); + + Assert.Equal( + @"new List(new string[] { ""one"" }) { ""one"", ""two"" }", + new CSharpHelper(typeMapping).UnknownLiteral(new SimpleTestType())); + } + [ConditionalFact] public void Literal_with_unsupported_node_throws() { From fd61793a4696e705315221b461ce52922f6fb0f9 Mon Sep 17 00:00:00 2001 From: Matt Morrissette Date: Fri, 24 Jun 2022 13:00:57 -0700 Subject: [PATCH 2/2] Fixes from review comments --- .../Design/Internal/CSharpHelper.cs | 53 ++++++------------- .../Design/Internal/CSharpHelperTest.cs | 41 ++++---------- 2 files changed, 26 insertions(+), 68 deletions(-) diff --git a/src/EFCore.Design/Design/Internal/CSharpHelper.cs b/src/EFCore.Design/Design/Internal/CSharpHelper.cs index c79d241af75..562fb412e2b 100644 --- a/src/EFCore.Design/Design/Internal/CSharpHelper.cs +++ b/src/EFCore.Design/Design/Internal/CSharpHelper.cs @@ -743,7 +743,7 @@ public virtual string Literal(object?[,] values) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual string Literal(IList values, bool vertical = false) + public virtual string Literal(List values, bool vertical = false) => List(typeof(T), values, vertical); private string List(Type type, IEnumerable values, bool vertical = false) @@ -752,13 +752,14 @@ private string List(Type type, IEnumerable values, bool vertical = false) builder.Append("new List<") .Append(Reference(type)) - .Append("> {"); + .Append(">"); var first = true; foreach (var value in values) { if (first) { + builder.Append(" {"); if (vertical) { builder.AppendLine(); @@ -787,17 +788,24 @@ private string List(Type type, IEnumerable values, bool vertical = false) builder.Append(UnknownLiteral(value)); } - if (vertical) + if (first) { - builder.AppendLine(); - builder.DecrementIndent(); + builder.Append("()"); } else { - builder.Append(" "); - } + if (vertical) + { + builder.AppendLine(); + builder.DecrementIndent(); + } + else + { + builder.Append(" "); + } - builder.Append("}"); + builder.Append("}"); + } return builder.ToString(); } @@ -948,35 +956,6 @@ private bool HandleExpression(Expression expression, StringBuilder builder, bool HandleList(((NewArrayExpression)expression).Expressions, builder, simple: true); - builder - .Append(" }"); - - return true; - case ExpressionType.ListInit: - var listExpr = (ListInitExpression)expression; - if (listExpr.Initializers.Any(i => i.Arguments.Count != 1)) - { - // If there is an initializer with more than one argument we can't make a literal cleanly - return false; - } - - builder - .Append("new ") - .Append(Reference(expression.Type)); - - if (listExpr.NewExpression.Arguments.Count > 0 && !HandleArguments(listExpr.NewExpression.Arguments, builder)) - { - return false; - } - - builder - .Append(" { "); - - if (!HandleList(listExpr.Initializers.Select(i => i.Arguments.First()), builder, simple: true)) - { - return false; - } - builder .Append(" }"); diff --git a/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs b/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs index be2ae861898..2fc765dab8f 100644 --- a/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs +++ b/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs @@ -125,13 +125,19 @@ public void Literal_works_when_many_ByteArray() public void Literal_works_when_empty_list() => Literal_works( new List(), - @"new List { }"); + @"new List()"); [ConditionalFact] - public void Literal_works_when_list_without_ctor_arguments() + public void Literal_works_when_list_with_single_element() => Literal_works( - new List { "one", "two" }, - @"new List { ""one"", ""two"" }"); + new List { "one" }, + @"new List { ""one"" }"); + + [ConditionalFact] + public void Literal_works_when_list_of_mixed_objects() + => Literal_works( + new List { 1, "two" }, + @"new List { 1, ""two"" }"); [ConditionalFact] public void Literal_works_when_list_with_ctor_arguments() @@ -625,33 +631,6 @@ public void Literal_with_add() new CSharpHelper(typeMapping).UnknownLiteral(new SimpleTestType())); } - [ConditionalFact] - public void Literal_with_list_init_without_ctor_arguments() - { - var typeMapping = CreateTypeMappingSource( - v => Expression.ListInit( - Expression.New(typeof(List<>).MakeGenericType(typeof(string))), - Expression.Constant("one"), Expression.Constant("two"))); - - Assert.Equal( - @"new List { ""one"", ""two"" }", - new CSharpHelper(typeMapping).UnknownLiteral(new SimpleTestType())); - } - - [ConditionalFact] - public void Literal_with_list_init_with_ctor_arguments() - { - var constructor = typeof(List<>).MakeGenericType((typeof(string))).GetConstructor(new[] { typeof(IEnumerable) })!; - var typeMapping = CreateTypeMappingSource( - v => Expression.ListInit( - Expression.New(constructor, Expression.NewArrayInit(typeof(string), Expression.Constant("one"))), - Expression.Constant("one"), Expression.Constant("two"))); - - Assert.Equal( - @"new List(new string[] { ""one"" }) { ""one"", ""two"" }", - new CSharpHelper(typeMapping).UnknownLiteral(new SimpleTestType())); - } - [ConditionalFact] public void Literal_with_unsupported_node_throws() {