[Feature] Add a ShadowAttribute for annotating fluent api

[jzabroski]

Currently, it is impossible to know from looking at code whether a HasForeignKey string value is referring to a shadow property or a class property. I've been thinking about how to make this more readable, and have an idea:

1. If mapping to a class property, use nameof(MyClass.ThePropertyId)
2. If intending for a shadow property, use [Shadow] "thePropertyId"

I think this not only makes the code easier to read, it resolves potential scenarios where the Metadata engine is trying to heuristically decide whether a string is a class property name or a database column name.

Benefits I see:

1. Better error messages become possible. The notion of a shadow property is reified directly in the API, so the API consumer can communicate to the framework its intentions, and the framework can in turn produce better warnings and errors when it cannot resolve those intentions.
2. Potential for Roslyn code analysis, since the only two semantically valid ways to call the API are the above specified.
3. Won't* break existing API for existing clients. (* denotes potential wishful thinking)

[smitpatel]

Metadata engine is trying to heuristically decide whether a string is a class property name or a database column name.

There is no heuristic. If you write a string name, if CLR property with that name exist we use it. If not, it's shadow. Reason: If you have CLR property, then we cannot define a shadow property with same name. EntityTypes cannot contain multiple properties with same name. If there is no CLR property then it has to be shadow only. Shadow is all about having backing CLR property or not. There is hardly any choice about user wanting to define the string property as shadow or not since it is controlled by CLR class. Now in the case, user wants to make sure that no accidental shadow property creeps in, using nameof syntax ensure that the string passed is having a corresponding CLR property.

[jzabroski]

There is no heuristic.

You say that, but then you define a heuristic:

If you write a string name, if CLR property with that name exist we use it. If not, it's shadow.

Note: I'm not trying to argue you with you, just providing feedback on my experience using database-first modeling and trying to get stuff working. The error messages that surface are confusing to me.

Now in the case, user wants to make sure that no accidental shadow property creeps in, using nameof syntax ensure that the string passed is having a corresponding CLR property.

Precisely my point. nameof operator is not a CLR metadata concept. You cannot reify it inside the EFCore Metadata engine. You cannot contractually guarantee no accidental property creeps in.

This is doubly confusing with TPH btw, because of the behavior to "guarantee safe name" gets generated. I've had tests that pass with EF Migrations-first approach, but fail with database-first approach. Perhaps I am not describing my problems sufficiently.

[jzabroski]

Here is an example that demonstrates the confusion - the other user literally filed the bug with confusing in the issue title: #5377 (comment)

[jzabroski]

I will target adding a minimal demo that demonstrates my pain next week.

[smitpatel]

There is difference between rule & heuristic. What we have is a rule and not heuristic.
EF does not do anything specific with nameof, it is more of a way for users to have compile time check to make sure they are not writing wrong property name. And the issue you linked has no relevance. It was about entityType name being incorrect rather than property name and we have fixed that issue about removing that API.
I would suggest you to file an actual issue of what you are seeing and what you are trying to do as feedback.

[jzabroski]

@smitpatel It's exactly my problem.

The issue is that since the API accepts a params string[] collection, and the overload introduced by @ajcvickers to provide syntactic sugar in #6003 , if you just write HasForeignKey("MyId") and forget to add the type parameter and use HasForeignKey<MyEntity>("MyId"), you get bad error messages that are confusing.

This is the third time in three weeks I've been tripped up by this. At this point I hope I don't forget this "heuristic"/"rule"/whatever again, but the bottom line is every time I hit these error messages I go searching Google trying to figure out what EF is trying to do.

[smitpatel]

Make sure you are not hitting #9171

Apart from that, if error message is confusing, I do not think it warrants even more confusing APIs for customers. The correct solution would be improve error message.

[jzabroski]

Fair enough - I was just getting my thoughts out, and this discussion helped me refine those thoughts. I will close this issue since I started a review on #6003 instead. (If that is the wrong workflow, I apologize - not trying to be a PITA.)

[jzabroski]

And yes, #9171 is exactly my issue. I see that it was closed, but don't quite see the fix in the PR. Andrew's comments suggest he thought that passing in a single string value was not a valid use case:

I wonder if we could just change it (probably in 3.0) to really have an single string overload just for the navigation name. This might be okay, because the times when you really want to only pass the single string type name seem rare to me--relationship with no navigation property where you don't want to use typeof() for some reason. Also, in this case there isn't a navigation property that matches, so after the change if there was code using this it could throw. It's hackish, but it could even warn and then drop back to using the name as a type if the navigation was not found--but not sure we even need to go that far.

The "rare" scenarios @ajcvickers is suggesting seem to be precisely the common scenario when doing database-first modeling. I don't see #9171 as actually fixed as per my comments here.

Maybe I am missing something.