Repository navigation
fix(engine): unwrap scalar PSObject during collection binding - #27970
Daksh Goyal (daksh-goyal) wants to merge 1 commit into
Conversation
Unwrap scalar values before adding them to collection parameters, matching the existing behavior for list elements. This prevents StringCollection parameters from attempting to cast a PSObject directly to a string. Add regression coverage for a single pipeline-produced value.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟢 Approval recommended
The binding fix is targeted and low-risk, and it is backed by a focused regression test covering the reported failure scenario.
Pull request overview
This PR fixes a parameter-binding edge case where a scalar PSObject (wrapping a string) was added directly to non-generic IList-backed collection parameters (e.g., StringCollection), causing invalid casts; it now unwraps the scalar value first, matching existing behavior for list elements.
Changes:
- Unwrap scalar values via
PSObject.Base(...)before adding them to collection parameters inParameterBinderBase.EncodeCollection. - Add a regression Pester test that reproduces the failure shape using a
StringCollectionparameter and a single pipeline-produced scalar value.
File summaries
| File | Description |
|---|---|
| test/powershell/Language/Parser/ParameterBinding.Tests.ps1 | Adds regression coverage for binding a scalar pipeline-produced PSObject to StringCollection. |
| src/System.Management.Automation/engine/ParameterBinderBase.cs | Unwraps scalar PSObject values prior to collection insertion to avoid invalid casts and align with element-binding behavior. |
Review details
Suppressed comments (1)
src/System.Management.Automation/engine/ParameterBinderBase.cs:1827
- The binding tracer format string uses
{1}but only supplies a single argument, so traces will literally print{1}instead of the element type. This makes diagnosing coercion-required cases harder.
if (currentValueElementType != desiredElementType &&
!currentValueElementType.IsSubclassOf(desiredElementType))
{
bindingTracer.WriteLine(
"COERCION REQUIRED: Did not coerce scalar arg value to type {1}",
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
microsoft-github-policy-service
Bot
added
the
Review - Needed
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
| "Argument type {0} is not IList, treating this as scalar", | ||
| currentValue.GetType().Name); | ||
|
|
||
| object currentValueElement = PSObject.Base(currentValue); |
There was a problem hiding this comment.
This applies to every collection type, and that is too broad. It's better to special case the StringCollection type as the WG comment pointed out in the issue.
|
This PR is superseded by #28141 |
PR Summary
Unwrap scalar values before adding them to collection parameters, matching the existing behavior for list elements. This prevents StringCollection parameters from attempting to cast a PSObject directly to a string.
Add regression coverage for a single pipeline-produced value.
PR Context
fixes #27969
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header