EF-76: Merge ProjectionBindingRemovingExpressionVisitor and MongoProjectionBindingRemovingExpressionVisitor #238
Conversation
…indingRemovingExpressionVisitor
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates two expression visitor classes by merging functionality from ProjectionBindingRemovingExpressionVisitor into MongoProjectionBindingRemovingExpressionVisitor and removing the abstract base class.
- Eliminated abstract base class
ProjectionBindingRemovingExpressionVisitor - Merged all functionality into the concrete
MongoProjectionBindingRemovingExpressionVisitorclass - Updated inheritance hierarchy to derive directly from
ExpressionVisitor
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ProjectionBindingRemovingExpressionVisitor.cs | Removed entire abstract base class file (627 lines deleted) |
| MongoProjectionBindingRemovingExpressionVisitor.cs | Merged functionality from base class, changed inheritance, and added all previously abstract methods |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| var concreteEntityTypeVariable = shaperBlock.Variables.Single(v => v.Type == typeof(IEntityType)); | ||
| var inverseNavigation = navigation.Inverse; | ||
| var fixup = GenerateFixup( | ||
| includingClrType, relatedEntityClrType, navigation, inverseNavigation!); |
There was a problem hiding this comment.
The null-forgiving operator (!) is used on inverseNavigation without proper null checking. This could lead to runtime exceptions if inverseNavigation is actually null. Consider adding proper null validation or handling the null case explicitly.
| includingClrType, relatedEntityClrType, navigation, inverseNavigation!); | |
| includingClrType, relatedEntityClrType, navigation, inverseNavigation); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
@damieng On this, what is the status of unidirectional relationships, where there is one direction but not the inverse? (Technically, relationships with no navigations are also supported, but that's edge.)
There was a problem hiding this comment.
We support unidirectional - we have to for owned entities to work.
There was a problem hiding this comment.
Think I'm going to leave this unchanged for now.
...ongoDB.EntityFrameworkCore/Query/Visitors/MongoProjectionBindingRemovingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
...ongoDB.EntityFrameworkCore/Query/Visitors/MongoProjectionBindingRemovingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
…BindingRemovingExpressionVisitor.cs Co-authored-by: Copilot <[email protected]>
…BindingRemovingExpressionVisitor.cs Co-authored-by: Copilot <[email protected]>
2 participants
No description provided.