Skip to content

Conversation

@Ravi-Raghavan
Copy link
Collaborator

Support Fully Qualified Names for Data Models

@Ravi-Raghavan Ravi-Raghavan requested a review from BorisDog June 27, 2024 18:55
Builders<Person>.Filter.Eq(p => p.Name, MongoDB.Analyzer.Tests.Common.DataModel.StaticHolder.ReadonlyString) |
Builders<Person>.Filter.Eq(p => p.Name, MongoDB.Analyzer.Tests.Common.DataModel.StaticHolder.ReadonlyString2) |
Builders<Person>.Filter.Eq(p => p.Name, MongoDB.Analyzer.Tests.Common.DataModel.StaticHolder.ReadonlyString) |
Builders<Person>.Filter.Eq(p => p.Vehicle.VehicleType.Type, MongoDB.Analyzer.Tests.Common.DataModel.StaticHolder.ReadonlyEnum) |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same naming pattern, no need to test all types.
Also consider combing Qualified_inside_method and Qualified_inside_namespace into single test with different naming nesting levels. Given A.B...X.Y.Class.Property is a full name, let's test:

A.B...X.Y.Class.Propert
B...X.Y.Class.Propert
X.Y.Class.Propert
Y.Class.Propert

Same for Builders<A.B...X.Y.Class>.

Also test cases like:

using user = MongoDB.Analyzer.Tests.Common.DataModel.User;
using driver = MongoDB.Driver;
_ = driver.Builders<user>.Filter.Eq(user => user.Age, 22);
_ = driver.Builders<MongoDB.Analyzer.Tests.Common.DataModel.User>.Filter.Eq(user => user.Age, 22);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

var isBuildersContainer = false;
var containsAlias = false;

//Check if Member Expression is Builders Container
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add space after // in all comments

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

syntaxNode = (syntaxNode as MemberAccessExpressionSyntax)?.Expression;
}

//Skip Nodes that are Builder Containers and that don't contain alias
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add two examples in the comment:
// For example MongoDB.Driver.Builders, driverAlias.Builders

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Collaborator

@BorisDog BorisDog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Good work!

@Ravi-Raghavan Ravi-Raghavan merged commit 626bb3f into mongodb:main Jul 2, 2024
@Ravi-Raghavan Ravi-Raghavan deleted the VS-146 branch July 2, 2024 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants