-
Notifications
You must be signed in to change notification settings - Fork 120
[feature/10.0] Replace reflection-based options mapping logic #8233
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
base: feature/10.0
Are you sure you want to change the base?
Conversation
Enumerable.Empty<PropertyInfo>(); | ||
public static IEnumerable<PropertyInfo> GetPropertiesFromSettings(Type type, Predicate<PropertyInfo>? predicate = null) | ||
{ | ||
return type.GetProperties(BindingFlags.Public | BindingFlags.Instance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify why this works correctly for Type.GetProperties but not object?.GetType().GetProperties?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible to annotate the Type
parameter with [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)]
, which will cause our tooling to keep public properties for types passed to this method (and warn for types that aren't statically known). I'm planning to actually add the trim annotations for this and other patterns in a separate PR.
The 'no-recent-activity' label has been added to this pull request due to four weeks without any activity. If there is no activity in the next six weeks, this pull request will automatically be closed. You can learn more about our stale PR policy here: https://github.com/dotnet/dotnet-monitor/blob/main/CONTRIBUTING.md#stale-pr-policy |
This replaces the reflection-based logic from CommonOptionsExtensions.cs with a statically-typed version.
The statically typed version could be source generated, but for now it is written by hand (with plenty help from copilot).
CommonOptionsExtensions has been moved into the test tree, and a new testcase has been added that:
Includes a fix to the mapping logic to treat
Uri
as a built-in type (calling ConvertUtils.ToString instead of recursing into its properties).