[release/8.0] Update binder gen emitted-interceptor nullability to match framework impl #91359
Conversation
layomia
added
Servicing-consider
|
Tagging subscribers to this area: @dotnet/area-extensions-configuration Issue DetailsBackport of #91180 to release/8.0 Customer ImpactUpdates null input handling by the generated replacement binding methods to align with the reflection-based implementation. This follows the general user expectation of consistency between the two implementations. TestingAutomated null handling tests have been added for all parameters of all binder methods. They assert the same behavior for source-gen and reflection. Some null refs in the reflection binder were fixed as well. This also provides complete build coverage for all binding inputs. RiskLow. It's a contained fix for an off-by-default component. cc @carlossanlop.
|
....Configuration.Binder/gen/Helpers/Emitter/OptionsConfigurationServiceCollectionExtensions.cs
Show resolved
Hide resolved
| // ----------------------------------------------------------------------------------------------------------------------------- | ||
| // | bindingPoint | bindingPoint | | ||
| // Interface | Value | IsReadOnly | Behavior | ||
| // ----------------------------------------------------------------------------------------------------------------------------- | ||
| // ISet<T> | not null | true/false | Use the Value instance to populate the configuration | ||
| // ISet<T> | null | false | Create HashSet<T> instance to populate the configuration | ||
| // ISet<T> | null | true | nothing | ||
| // IReadOnlySet<T> | null/not null | false | Create HashSet<T> instance, copy over existing values, and populate the configuration | ||
| // IReadOnlySet<T> | null/not null | true | nothing | ||
| // ----------------------------------------------------------------------------------------------------------------------------- |
There was a problem hiding this comment.
These comments with the table of scenarios is really helpful
There was a problem hiding this comment.
Yeah the source gen impl followed this closely.
src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs
Show resolved
Hide resolved
jeffhandley
added
Servicing-approved
ghost
locked as resolved and limited conversation to collaborators
Backport of #91180 to release/8.0
Fixes #90987 for 8.0.
Customer Impact
Updates null input handling by the generated replacement binding methods to align with the reflection-based implementation. This follows the general user expectation of consistency between the two implementations.
Testing
Automated null handling tests have been added for all parameters of all binder methods. They assert the same behavior for source-gen and reflection. Some null refs in the reflection binder were fixed as well. This also provides complete build coverage for all binding inputs.
Risk
Low. It's a contained fix for an off-by-default component.
cc @carlossanlop.