Fixed codegen for IEnumerable<T> binding (#123422) #123663
Conversation
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-extensions-configuration |
|
@dotnet-policy-service agree company="Growings RobotX Ltd." |
There was a problem hiding this comment.
Pull request overview
This pull request fixes a regression in the configuration binder's source generator where binding IEnumerable<T> in nested positional records incorrectly threw an InvalidOperationException. The issue occurred because the code generation logic for handling empty collections and error conditions only checked for ArraySpec types, but not for IEnumerable<T> types.
Changes:
- Updated the condition for emitting exception-throwing code to handle both
ArraySpecand types withIsExactIEnumerableOfTproperty - Added a test case to verify that nested
IEnumerable<T>in positional records can be bound successfully
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs | Fixed the condition that determines when to emit if (member is null) vs else for exception throwing, now including IEnumerable<T> types alongside arrays |
| src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs | Added test case TestBindingNestedIEnumerable to verify the fix works for the reported scenario |
| src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs | Added test classes ContainingIEnumerable and NestedWithIEnumerable to support the new test case |
...libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs
Show resolved
Hide resolved
dmitry-kandiner
force-pushed
the
bugfix/issue-#123422
branch
2 times, most recently
from
88da365 to
ee73a21
Compare
jkotas
changed the title
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 dotnet#123422 Signed-off-by: Dmitry Kandiner <dkandiner@growings.com>
Signed-off-by: Dmitry Kandiner <dkandiner@growings.com>
dmitry-kandiner
force-pushed
the
bugfix/issue-#123422
branch
from
ee73a21 to
c23c554
Compare
|
@dmitry-kandiner please don't force push again because this reset the review. Thanks! |
Oops! I assumed not being lined up to the target branch will prevent merging. My mistake. |
No worries. We don't have to do that except if there is a merge conflict. Otherwise, every change in the PR will reset the CI run and takes longer to finish. |
|
@tarekgh could you please trigger re-build of the timed out tests? |
|
/azp run |
|
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list. |
Supported commands
See additional documentation. |
|
/azp run runtime |
|
Commenter does not have sufficient privileges for PR 123663 in repo dotnet/runtime |
|
@dmitry-kandiner don't worry about the CI and don't have to merge your branch to main as you did, Anyway, I'll follow up and get this merged. I just need to do some local testing before merging it. Thanks again! |
|
@tarekgh Thank you! Apparently, the checks fail due to timeout, but I have no means to check what's going on exactly. |
|
Yes, the timeout is known issue to us. CC @markwilkie |
3 participants
Binding of an IEnumerable 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 as well.
Fix #123422