RevEng: Use nameof in data annotations

[ravensorb]

Currently when creating an ef view caffolding with attributes enabled, all of the attributes are using hard coded strings from the properties names of the related entity. This is very brittle and can make changes in the entity model extremely difficult and error prone. The request is to change the code generation logic to output attributes that leverage nameof instead to address this issue.

Further technical details

EF Core version: 2.1.0-preview2-final
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: Visual Studio 2017 15.4

[bricelam]

Meta question for the team: What is the lowest version of C# we need to support?

[ErikEJ]

C# 6 is my vote

[ajcvickers]

Discussed in triage and decided that we will not do this yet because we still support people using EF Core with .NET Framework on earlier versions of VS.

[ravensorb]

So this is more of a compiler issue than a framework issue?

[bricelam]

Yes, we can't generate nameof() unless everywhere you can use EF Core is able to compile it.

[ravensorb]

What about making this a setting that can be enabled/disabled by the called (or the IDE/Command line)?

[ajcvickers]

Removing from backlog temporarily to discuss the command line switch.

[ajcvickers]

@ravensorb We discussed this and came to the conclusion that:

• It would be good to add a "language version" in parallel with the currently supplied "language".
• This should be read from the user's project by default, just like the language is.
• Based on this, we can start using features that are supported by the language version being used.

Marking this a up-for-grabs since this should not be too difficult to implement--advice from @bricelam is to "follow the language" code.

[ravensorb]

@ajcvickers this sounds like an excellent solution! I am looking forward to it :)

[Muppets]

I was going to take a look at this but wanted to confirm which code generation we are looking to update.

Running this command:

Scaffold-DbContext "Server=(localdb)\mssqllocaldb;Database=Northwind;Trusted_Connection=True;" Microsoft.EntityFrameworkCore.SqlServer -DataAnnotations -OutputDir Models

This creates models that include relationship properties annotated like so:

        [ForeignKey("CategoryId")]
        [InverseProperty("Products")]
        public virtual Categories Category { get; set; }

Updating this to use nameof, we would want something like:

        [ForeignKey(nameof(CategoryId))]
        [InverseProperty(nameof(Categories.Products))]
        public virtual Categories Category { get; set; }

Is this the right changes we are talking about here? Any other annotations that would need updating?

[bricelam]

That sounds right.

We also need to investigate what maximum version of C# we can use. But don’t let that stop you from submitting a PR.

[andreujuanc]

I would love to do this. I'm migrating a webapp that uses EF 6 to EF Core, and the DB and it's edmx were a mess, so we decided to redo the entities via scaffold. We had to rename some stuff and BOM, InverseProperty names are hardcoded :( Wish it had nameof

This would have saved me couple of hours on runtime validation.

[ErikEJ]

@bricelam Can I do this now, that .NET Standard 2.1 is minimum for EF Core 3.0?

[bricelam]

@Muppets already has an open PR. But yes, I think we can move forward with this since C# 8.0 will be the default for projects targeting .NET Standard 2.1 and .NET Core 3.0.

[ErikEJ]

@bricelam Sorry, missed that

[Varorbc]

@bricelam TableAttribute miss nameof, can you add it?

[bricelam]

@Varorbc Personally, I don't think it belongs there since the table name is independent of the type name. Note that the places it was added ([ForeignKey] and [InverseProperty]) refer to CLR type and property names, not table and column names.

[bricelam]

One important scenario that would be lost by using nameof in [Table]:

After scaffolding, we want you to easily rename your CLR types but still have your model map to the same database schema. If we used nameof, it would also change the table mapping.