Skip to content

Commit 1a62dde

Browse files
AndriySvyrydAndriySvyryd
authored
Fix several small issues in the compiled model (#38441)
Handle discriminators that use a value converter in the compiled model Handle negative discriminators values in the compiled model Handle global namespaces in the compiled model Avoid variable-name clashes in lambdas in the compiled model Match UnsafeAccessors accessibility to the target type Fixes #37259 Fixes #35142 Fixes #35142 Fixes #35011 Fixes #35013
1 parent 6cfaca0 commit 1a62dde

21 files changed

Lines changed: 241 additions & 28 deletions

File tree

‎src/EFCore.Design/Design/Internal/CSharpHelper.cs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -989,10 +989,20 @@ public virtual string Literal(Enum value, bool fullName = false)
989989
return name == null
990990
? type.IsDefined(typeof(FlagsAttribute), false)
991991
? GetCompositeEnumValue(type, value, fullName)
992-
: $"({Reference(type)}){UnknownLiteral(Convert.ChangeType(value, Enum.GetUnderlyingType(type)))}"
992+
: GetUnnamedEnumValue(type, value)
993993
: GetSimpleEnumValue(type, name, fullName);
994994
}
995995

996+
private string GetUnnamedEnumValue(Type type, Enum value)
997+
{
998+
var underlyingLiteral = UnknownLiteral(Convert.ChangeType(value, Enum.GetUnderlyingType(type)));
999+
1000+
// A negative value must be enclosed in parentheses, otherwise e.g. (MyEnum)-1 fails to compile (CS0075).
1001+
return underlyingLiteral.StartsWith('-')
1002+
? $"({Reference(type)})({underlyingLiteral})"
1003+
: $"({Reference(type)}){underlyingLiteral}";
1004+
}
1005+
9961006
/// <summary>
9971007
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
9981008
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -1026,7 +1036,7 @@ protected virtual string GetCompositeEnumValue(Type type, Enum flags, bool fullN
10261036
previous == null
10271037
? GetSimpleEnumValue(type, Enum.GetName(type, current)!, fullName)
10281038
: previous + " | " + GetSimpleEnumValue(type, Enum.GetName(type, current)!, fullName))
1029-
?? $"({Reference(type)}){UnknownLiteral(Convert.ChangeType(flags, Enum.GetUnderlyingType(type)))}";
1039+
?? GetUnnamedEnumValue(type, flags);
10301040
}
10311041

10321042
internal static IReadOnlyCollection<Enum> GetFlags(Enum flags)

‎src/EFCore.Design/Query/Internal/LinqToCSharpSyntaxTranslator.cs‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,33 @@ protected virtual SyntaxNode TranslateCore(
140140
_context = statementContext ? ExpressionContext.Statement : ExpressionContext.Expression;
141141
_onLastLambdaLine = true;
142142

143-
Visit(node);
143+
// The constant replacements (e.g. variables declared in the enclosing generated method for properties,
144+
// navigations, etc.) are in scope for the whole translation, so register their names in the root stack frame.
145+
// This keeps the in-scope variable tracking accurate, ensuring generated variables and lambda parameters don't
146+
// clash with them (their references are emitted by name inside the translated lambda bodies).
147+
var rootFrame = _stack.Peek();
148+
if (constantReplacements != null)
149+
{
150+
foreach (var name in constantReplacements.Values)
151+
{
152+
rootFrame.VariableNames.Add(name);
153+
}
154+
}
155+
156+
try
157+
{
158+
Visit(node);
159+
}
160+
finally
161+
{
162+
if (constantReplacements != null)
163+
{
164+
foreach (var name in constantReplacements.Values)
165+
{
166+
rootFrame.VariableNames.Remove(name);
167+
}
168+
}
169+
}
144170

145171
if (_liftedState.Statements.Count > 0
146172
&& _context == ExpressionContext.Expression)
@@ -1491,6 +1517,12 @@ protected override Expression VisitLambda<T>(Expression<T> lambda)
14911517
foreach (var parameter in lambda.Parameters)
14921518
{
14931519
var name = parameter.Name ?? "unnamed" + (++localUnnamedParameterCounter);
1520+
1521+
if (_constantReplacements?.Values.Contains(name) == true)
1522+
{
1523+
name = UniquifyVariableName(name);
1524+
}
1525+
14941526
stackFrame.Variables[parameter] = name;
14951527
stackFrame.VariableNames.Add(name);
14961528
}

‎src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs‎

Lines changed: 75 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,24 @@ private string CreateAssemblyAttributes(
201201
private string GetModelClassName(Type contextType)
202202
=> _code.Identifier(contextType.ShortDisplayName()) + ModelSuffix;
203203

204+
private static string GetAccessibilityModifier(Type type)
205+
=> IsPubliclyAccessible(type) ? "public" : "internal";
206+
207+
private static bool IsPubliclyAccessible(Type type)
208+
{
209+
while (type.IsNested)
210+
{
211+
if (!type.IsNestedPublic)
212+
{
213+
return false;
214+
}
215+
216+
type = type.DeclaringType!;
217+
}
218+
219+
return type.IsPublic;
220+
}
221+
204222
private string GenerateUnsafeAccessorType(
205223
Type type,
206224
HashSet<MemberInfo> members,
@@ -222,7 +240,7 @@ private string GenerateUnsafeAccessorType(
222240
}
223241

224242
mainBuilder
225-
.Append("public static class ").Append(className);
243+
.Append(GetAccessibilityModifier(type)).Append(" static class ").Append(className);
226244
if (type.IsGenericTypeDefinition)
227245
{
228246
var genericParameters = type.GetGenericArguments();
@@ -715,7 +733,7 @@ private string GenerateEntityType(
715733
var className = entityClassNames[entityType];
716734
mainBuilder
717735
.AppendLine("[EntityFrameworkInternal]")
718-
.Append("public partial class ").AppendLine(className)
736+
.Append(GetAccessibilityModifier(entityType.ClrType)).Append(" partial class ").AppendLine(className)
719737
.AppendLine("{");
720738
using (mainBuilder.Indent())
721739
{
@@ -935,11 +953,15 @@ private void Create(IEntityType entityType, CSharpRuntimeAnnotationCodeGenerator
935953
var discriminatorValue = entityType.GetDiscriminatorValue();
936954
if (discriminatorValue != null)
937955
{
938-
AddNamespace(discriminatorValue.GetType(), parameters.Namespaces);
956+
var discriminatorConverter = entityType.FindDiscriminatorProperty()?.FindTypeMapping()?.Converter;
957+
if (discriminatorConverter == null)
958+
{
959+
AddNamespace(discriminatorValue.GetType(), parameters.Namespaces);
939960

940-
mainBuilder.AppendLine(",")
941-
.Append("discriminatorValue: ")
942-
.Append(_code.UnknownLiteral(discriminatorValue));
961+
mainBuilder.AppendLine(",")
962+
.Append("discriminatorValue: ")
963+
.Append(_code.UnknownLiteral(discriminatorValue));
964+
}
943965
}
944966

945967
var derivedTypesCount = entityType.GetDirectlyDerivedTypes().Count();
@@ -1030,6 +1052,26 @@ private void Create(IEntityType entityType, CSharpRuntimeAnnotationCodeGenerator
10301052
.AppendLine(");")
10311053
.AppendLine()
10321054
.DecrementIndent();
1055+
1056+
if (discriminatorValue != null)
1057+
{
1058+
var discriminatorConverter = entityType.FindDiscriminatorProperty()?.FindTypeMapping()?.Converter;
1059+
if (discriminatorConverter != null)
1060+
{
1061+
var providerValue = discriminatorConverter.ConvertToProvider(discriminatorValue);
1062+
if (providerValue != null)
1063+
{
1064+
AddNamespace(providerValue.GetType(), parameters.Namespaces);
1065+
}
1066+
1067+
mainBuilder
1068+
.Append(parameters.TargetName)
1069+
.Append(".SetDiscriminatorValueFromProviderValue(")
1070+
.Append(_code.UnknownLiteral(providerValue))
1071+
.AppendLine(");")
1072+
.AppendLine();
1073+
}
1074+
}
10331075
}
10341076

10351077
private void Create(
@@ -2353,11 +2395,15 @@ private void CreateComplexProperty(
23532395
var discriminatorValue = complexType.GetDiscriminatorValue();
23542396
if (discriminatorValue != null)
23552397
{
2356-
AddNamespace(discriminatorValue.GetType(), parameters.Namespaces);
2398+
var discriminatorConverter = complexType.FindDiscriminatorProperty()?.FindTypeMapping()?.Converter;
2399+
if (discriminatorConverter == null)
2400+
{
2401+
AddNamespace(discriminatorValue.GetType(), parameters.Namespaces);
23572402

2358-
mainBuilder.AppendLine(",")
2359-
.Append("discriminatorValue: ")
2360-
.Append(_code.UnknownLiteral(discriminatorValue));
2403+
mainBuilder.AppendLine(",")
2404+
.Append("discriminatorValue: ")
2405+
.Append(_code.UnknownLiteral(discriminatorValue));
2406+
}
23612407
}
23622408

23632409
mainBuilder.AppendLine(",")
@@ -2381,6 +2427,25 @@ private void CreateComplexProperty(
23812427
.Append("var ").Append(complexTypeVariable).Append(" = ")
23822428
.Append(complexPropertyVariable).AppendLine(".ComplexType;");
23832429

2430+
if (discriminatorValue != null)
2431+
{
2432+
var discriminatorConverter = complexType.FindDiscriminatorProperty()?.FindTypeMapping()?.Converter;
2433+
if (discriminatorConverter != null)
2434+
{
2435+
var providerValue = discriminatorConverter.ConvertToProvider(discriminatorValue);
2436+
if (providerValue != null)
2437+
{
2438+
AddNamespace(providerValue.GetType(), parameters.Namespaces);
2439+
}
2440+
2441+
mainBuilder
2442+
.Append(complexTypeVariable)
2443+
.Append(".SetDiscriminatorValueFromProviderValue(")
2444+
.Append(_code.UnknownLiteral(providerValue))
2445+
.AppendLine(");");
2446+
}
2447+
}
2448+
23842449
var complexTypeParameters = parameters with { TargetName = complexTypeVariable };
23852450
var complexPropertyParameters = parameters with { TargetName = complexPropertyVariable };
23862451

‎src/EFCore/Metadata/RuntimeTypeBase.cs‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ public abstract class RuntimeTypeBase : RuntimeAnnotatableBase, IRuntimeTypeBase
2020
private RuntimeModel _model;
2121
private readonly RuntimeTypeBase? _baseType;
2222
private SortedSet<RuntimeTypeBase>? _directlyDerivedTypes;
23-
private readonly object? _discriminatorValue;
23+
private object? _discriminatorValue;
24+
private object? _discriminatorValueFromProviderValue;
2425
private readonly Utilities.OrderedDictionary<string, RuntimeProperty> _properties;
2526
private Utilities.OrderedDictionary<string, RuntimeComplexProperty>? _complexProperties;
2627
private readonly PropertyInfo? _indexerPropertyInfo;
@@ -107,6 +108,16 @@ protected RuntimeTypeBase(
107108
public virtual RuntimeTypeBase? BaseType
108109
=> _baseType;
109110

111+
/// <summary>
112+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
113+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
114+
/// any release. You should only use it directly in your code with extreme caution and knowing that
115+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
116+
/// </summary>
117+
[EntityFrameworkInternal]
118+
public virtual void SetDiscriminatorValueFromProviderValue(object? value)
119+
=> _discriminatorValueFromProviderValue = value;
120+
110121
/// <summary>
111122
/// Gets all types in the model that directly derive from this type.
112123
/// </summary>
@@ -917,7 +928,21 @@ IModel ITypeBase.Model
917928
/// <inheritdoc />
918929
[DebuggerStepThrough]
919930
object? IReadOnlyTypeBase.GetDiscriminatorValue()
920-
=> _discriminatorValue;
931+
{
932+
var providerValue = _discriminatorValueFromProviderValue;
933+
if (providerValue != null)
934+
{
935+
var converter = ((ITypeBase)this).FindDiscriminatorProperty()?.GetTypeMapping().Converter;
936+
Interlocked.CompareExchange(
937+
ref _discriminatorValue,
938+
converter == null ? providerValue : converter.ConvertFromProvider(providerValue),
939+
null);
940+
941+
_discriminatorValueFromProviderValue = null;
942+
}
943+
944+
return _discriminatorValue;
945+
}
921946

922947
/// <inheritdoc />
923948
[DebuggerStepThrough]

‎src/Shared/SharedTypeExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,7 @@ public static IEnumerable<string> GetNamespaces(this Type type)
625625
}
626626
else
627627
{
628-
if (type.Namespace is not null)
628+
if (!string.IsNullOrEmpty(type.Namespace))
629629
{
630630
yield return type.Namespace;
631631
}

‎test/EFCore.Cosmos.FunctionalTests/Scaffolding/Baselines/BigModel/DependentBaseEntityType.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,14 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas
3131
typeof(CompiledModelTestBase.DependentBase<byte?>),
3232
baseEntityType,
3333
discriminatorProperty: "EnumDiscriminator",
34-
discriminatorValue: CompiledModelTestBase.Enum1.One,
3534
derivedTypesCount: 1,
3635
propertyCount: 6,
3736
navigationCount: 1,
3837
foreignKeyCount: 2,
3938
keyCount: 1);
4039

40+
runtimeEntityType.SetDiscriminatorValueFromProviderValue(1);
41+
4142
var principalId = runtimeEntityType.AddProperty(
4243
"PrincipalId",
4344
typeof(long),

‎test/EFCore.Cosmos.FunctionalTests/Scaffolding/Baselines/BigModel/DependentDerivedEntityType.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas
2727
typeof(CompiledModelTestBase.DependentDerived<byte?>),
2828
baseEntityType,
2929
discriminatorProperty: "EnumDiscriminator",
30-
discriminatorValue: CompiledModelTestBase.Enum1.Two,
3130
propertyCount: 2);
3231

32+
runtimeEntityType.SetDiscriminatorValueFromProviderValue(2);
33+
3334
var data = runtimeEntityType.AddProperty(
3435
"Data",
3536
typeof(string),

‎test/EFCore.Cosmos.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/DependentBaseEntityType.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,14 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas
2424
typeof(CompiledModelTestBase.DependentBase<byte?>),
2525
baseEntityType,
2626
discriminatorProperty: "EnumDiscriminator",
27-
discriminatorValue: CompiledModelTestBase.Enum1.One,
2827
derivedTypesCount: 1,
2928
propertyCount: 6,
3029
navigationCount: 1,
3130
foreignKeyCount: 2,
3231
keyCount: 1);
3332

33+
runtimeEntityType.SetDiscriminatorValueFromProviderValue(1);
34+
3435
var principalId = runtimeEntityType.AddProperty(
3536
"PrincipalId",
3637
typeof(long),

‎test/EFCore.Cosmos.FunctionalTests/Scaffolding/Baselines/No_NativeAOT/DependentDerivedEntityType.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas
2020
typeof(CompiledModelTestBase.DependentDerived<byte?>),
2121
baseEntityType,
2222
discriminatorProperty: "EnumDiscriminator",
23-
discriminatorValue: CompiledModelTestBase.Enum1.Two,
2423
propertyCount: 2);
2524

25+
runtimeEntityType.SetDiscriminatorValueFromProviderValue(2);
26+
2627
var data = runtimeEntityType.AddProperty(
2728
"Data",
2829
typeof(string),

‎test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,13 @@ public void UnknownLiteral_throws_when_unknown()
272272
Assert.Equal(DesignStrings.UnknownLiteral(typeof(object)), ex.Message);
273273
}
274274

275+
[Theory]
276+
[InlineData((SomeEnum)(-1), "(CSharpHelperTest.SomeEnum)(-1)")]
277+
[InlineData((SomeEnum)2, "(CSharpHelperTest.SomeEnum)2")]
278+
[InlineData((SomeSByteEnum)(-1), "(CSharpHelperTest.SomeSByteEnum)(sbyte)-1")]
279+
public void Literal_works_when_enum_value_is_unnamed(object value, string expected)
280+
=> Assert.Equal(expected, new CSharpHelper(TypeMappingSource).UnknownLiteral(value));
281+
275282
[Theory, InlineData(typeof(int), "int"), InlineData(typeof(int?), "int?"), InlineData(typeof(int[]), "int[]"),
276283
InlineData(typeof(int[,]), "int[,]"), InlineData(typeof(int[][]), "int[][]"), InlineData(typeof(Generic<int>), "Generic<int>"),
277284
InlineData(typeof(Nested), "CSharpHelperTest.Nested"), InlineData(typeof(Generic<Generic<int>>), "Generic<Generic<int>>"),
@@ -294,6 +301,11 @@ private enum SomeEnum
294301
Default
295302
}
296303

304+
private enum SomeSByteEnum : sbyte
305+
{
306+
Default
307+
}
308+
297309
[Theory, InlineData("dash-er", "dasher"), InlineData("params", "@params"), InlineData("true", "@true"),
298310
InlineData("yield", "yield"), InlineData("spac ed", "spaced"), InlineData("1nders", "_1nders"), InlineData("name.space", "@namespace"),
299311
InlineData("$", "_")]

0 commit comments

Comments
 (0)