-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remaining reference nullability work #16440
Comments
Some first things that we need to look into: Preconditions and postconditions ([AllowNull], [DisallowNull], [MaybeNull], [NotNull], see https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-05-15.md). These can go on:
|
Question, is there a document or discussion that explains how "string" vs "string?" will be treated by EF Core once non-nullable reference types are available, and how the [Required] attribute will be interpreted for each of them? |
@sjb-sjb we don't have anything documented yet AFAIK, but if you opt into the nullable reference types feature, non-nullable types (e.g. string) will be treated as if they have |
Do you mean that if I have string? then I can also add [Required] to force it to be non-nullable in the database -- in other words if I opt in to non-nullable reference types then "[Required] string?" and just "string" would be treated the same way? In that case the meaning of just plain "string" would change in both the EF database mapping and in C# when I opt into non-nullable reference types -- which would make sense. |
That's correct - as far as EF and the database are concerned. A non-nullable database column would be created in migrations, and EF would expect to never see a null when writing entities. |
@roji To find link to remaining change. |
Possible (but not certain) further changes in the nullability attributes: https://github.com/dotnet/corefx/issues/36222#issuecomment-511973898. Apart from that there's also pre- and post-conditions. |
Got confirmation that no further changes to nullability metadata. Keeping open to track pre- and post-conditions. |
As per dotnet/roslyn#37610, the compiler will no longer emit nullability context attributes at the module level, only at the type and method level. Simplify our non-nullability detection convention. Part of #16440
Note: the nullability context attribute will no longer appear at the module level (dotnet/roslyn#37610), so simplified our convention to not handle that. |
How would I specify a string that cannot be null but which can be empty ("")? If non-nullable string is interpreted by EF in the same way as [Required] string? then it would have to be a string of length at least one. Possibly-empty but non-null strings could be desirable for eliminating a lot of null-pointer tests after materialization. |
@sjb-sjb I think you're confusing what |
Got it, thanks. |
As per dotnet/roslyn#37610, the compiler will no longer emit nullability context attributes at the module level, only at the type and method level. Simplify our non-nullability detection convention. Part of dotnet#16440
As per dotnet/roslyn#37610, the compiler will no longer emit nullability context attributes at the module level, only at the type and method level. Simplify our non-nullability detection convention. Part of #16440
Here is a note regarding pre- and post-conditions (see these docs). In a nutshell, there are four attributes that allow you to override the default nullability setting on a get/set basis (e.g. a non-nullable property can still return or be set to null). Within the EF Core context, I thought about it a little bit and came up with the following spec.
According to the table above, the only attributes EF actually needs to recognize is [MaybeNull], since it means that a non-nullable property may actually return nulls (which need to be written to the database). This logic is implemented in #16985, but it would be good to confirm the above. |
Decision: like with other modelling aspects, we only consult the public-facing property, so the field's reference nullability will not be taken into account when deciding on the EF property's requiredness. If only a field is configured without a property, the field's nullability will of course be the determining factor. This means that users can make EF aware only of the field, and do whatever they want with the property. |
@roji Agree with your analysis/decisions on the pre/post conditions. |
As per dotnet/roslyn#37610, the compiler will no longer emit nullability context attributes at the module level, only at the type and method level. Simplify our non-nullability detection convention. Part of #16440
Closing as [MaybeNull] has been handled in #16985, and we've decided not to look at backing fields for the purpose of requiredness. |
@ajcvickers, on your note that [Required] on a nullable property causes EF to make the database field non-nullable .... does it have to be RequiredAttribute or could it be an attribute derived from RequiredAttribute ? |
@sjb-sjb Might work with a derived type. Have you tried it? |
We are going to use this issue to track any additional changes necessary to properly support nullable reference types in 3.0, in particular react to any API changes to the C# 8.0 compiler generated attributes and support for custom attributes.
The text was updated successfully, but these errors were encountered: