-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[release/10.0] Fixed codegen for IEnumerable<T> binding (#123422) #123720
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
Conversation
Binding of an IEnumerable<T> in a nested positional record resulted in throwing of an InvalidOperationException from source generated code. The condition for emitting the code throwing the exception only worked with Arrays, and this fix applies the same logic for IEnumerable<T> as well. Fix #123422 Signed-off-by: Dmitry Kandiner <dkandiner@growings.com>
Signed-off-by: Dmitry Kandiner <dkandiner@growings.com>
|
Tagging subscribers to this area: @dotnet/area-system-configuration |
ericstj
left a comment
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.
The change looks minimal and addresses the reported regression. As before these special cases make me worry if we have inconsistent behavior around collection types.
...crosoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs
Show resolved
Hide resolved
artl93
left a comment
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.
Customer reported regression in 10. Narrow fix. Completed variant analysis. Approved.
|
Is there any chance for adding this fix to 10.0.x milestone? |
What do you mean by that? This PR is about that. It is to merge the change to the release/10.0 branch for servicing. |
|
Approved offline by email. |
|
/ba-g the native AOT arm failing tests are known and unrelated. The infrastructure team is already following up. |
Backport of #123663 to release/10.0
/cc @tarekgh @dmitry-kandiner
Customer Impact
Using configuration source generator, when a user binds configuration to a class or record with a primary constructor, and one of the constructor parameters is exact of type
IEnumerable<T>, the binding throws aSystem.InvalidOperationException, even though the binding is expected to succeed. The issue #123422 shows the customer report and more details.Regression
This is a regression in .NET 10.0.1. A previous servicing fix addressed several crashes, but one case was missed. That omission surfaced as this regression. The change that introduced the regression is #121325.
Testing
Manually verified the failing scenario and confirmed the fix resolves the issue. Added a regression test covering the previously failing case and ran the full test suite successfully. Reviewed related code paths to ensure no additional cases were missed.
Risk
Low, the change is targeting the specific case with specific type. The fix is localized to the failing scenario.