Skip to content
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

Enable nullability in migrations and model snapshots #18950

Open
Tracked by #22946
TorreyGarland opened this issue Nov 17, 2019 · 12 comments
Open
Tracked by #22946

Enable nullability in migrations and model snapshots #18950

TorreyGarland opened this issue Nov 17, 2019 · 12 comments
Assignees
Labels
area-migrations customer-reported punted-for-6.0 punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-enhancement
Milestone

Comments

@TorreyGarland
Copy link

I am generating migrations files for EF core with the new nullable reference types analysis turned on for an ASP.NET core app (3.0).

The migration file generated has several "new object[]" declarations that VS is flagging for CS8625 warnings (screenshot included).

image

Is there a work-around or change coming so that we don't have to do a global suppression. I want the nullable reference analysis, but just not for these migration files.

@ajcvickers ajcvickers added this to the Backlog milestone Nov 18, 2019
@ajcvickers
Copy link
Member

@TorreyGarland Putting this in the backlog to consider in the future. The global suppression seems like the best workaround for now.

@Timovzl
Copy link

Timovzl commented Nov 30, 2020

@ajcvickers @TorreyGarland Could either of you provide an example of such a global supression? I'm trying to get them to work, but with no success.

[assembly: SuppressMessage("Code Quality", "CS8625:Cannot convert null literal to non-nullable reference type.",
Justification = "EF migrations.",
Scope = "NamespaceAndDescendants", Target = "~N:My.Namespace.Migrations")]

I did read somewhere that the NamespaceAndDescendants scope was broken in VS 16.7, and that it would be fixed in 16.8. I'm now on 16.8.2, but the suppression is not helping.

@roji
Copy link
Member

roji commented Nov 30, 2020

I think that by global suppression, it was meant that you don't activate the NRT feature at the project level (i.e. <Nullable>disabled</Nullable>). You can place your migrations in a separate project to keep nullability enabled in your actual code, if you want.

Another option is to place a #nullable disable at the top of each migration file.

@roji roji removed this from the Backlog milestone Nov 30, 2020
@roji
Copy link
Member

roji commented Nov 30, 2020

Clearing milestone to discuss if we want to do something for 6.0.

@Timovzl
Copy link

Timovzl commented Nov 30, 2020

@roji I'm pretty sure global suppressions is generally the term for what I mentioned above, so I assume that's what @TorreyGarland meant. I can't see why it's not working, though.

Disabling nullable for the entire project is too impactful, unfortunately. Adding #nullable disable at the top of each migration file would work, as would disabling the warnings from those locations.

I'm still hoping for a solution that makes the problem go away, instead of requiring each developer to remember for each new migration. We have "treat warnings as errors" enabled in the DevOps pipeline, to prevent the solution from becoming riddled with warnings... so it's quite a nuisance. 😛

@roji
Copy link
Member

roji commented Nov 30, 2020

You're right - global suppressions should indeed also work, I'm not sure why the above doesn't. I also agree we should provide a better solution than requiring developers to handle this themselves every time.

@maxkoshevoi
Copy link

maxkoshevoi commented Aug 19, 2021

@roji How about adding #nullable disable at the top of each migration automatically as part of it's generation? At least untill a proper fix will be ready.
Should be relatively easy to implement and would solve the problem completely without side effects.

@roji
Copy link
Member

roji commented Aug 19, 2021

Yeah, that's what we did for scaffolding before 6.0. We should consider doing this (both for migrations and for the model snapshot).

@roji
Copy link
Member

roji commented Aug 20, 2021

Opened #25624 to track adding #nullable disable, keeping this to track generating proper NRT code in the future.

@jacobjmarks

This comment has been minimized.

@roji

This comment has been minimized.

@roji roji changed the title Migrations and Nullable Reference Types Enable nullability in migrations and model snapshots Oct 12, 2021
@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0 Oct 25, 2021
@roji
Copy link
Member

roji commented Jan 24, 2022

Scenarios:

@ajcvickers ajcvickers added propose-punt punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. and removed propose-punt labels Jul 6, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0, Backlog Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations customer-reported punted-for-6.0 punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-enhancement
Projects
None yet
Development

No branches or pull requests

7 participants