fix: PropertySet argument transform crash when -PropertySet omitted - #88
Merged
Conversation
Test-FeatureFlag forwards -PropertySet into Test-Condition (which already requires it) via a splat. When -PropertySet was omitted, that forwarded $null was an explicit bind, which fired PropertySetTransformAttribute.Transform(). Its null branch called the argument-less Read-PropertySet, which requires -Name or -FilePath and can never bind, so it always threw a confusing transform error instead of a normal missing-parameter message. - Transform() now returns $null on null input instead of calling the broken, module-function-from-a-class-method Read-PropertySet. - Test-FeatureFlag's -PropertySet is now Mandatory, matching Test-Condition, so a missing value fails fast with PowerShell's standard mandatory-parameter error. Fixes #84
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a PowerShell argument transformation crash when Test-FeatureFlag is called without -PropertySet, by avoiding transform-time calls into Read-PropertySet and by making the missing-argument failure surface as PowerShell’s standard mandatory-parameter error.
Changes:
- Updated
PropertySetTransformAttribute.Transform()to return$nullfor null input instead of callingRead-PropertySetwith no arguments. - Made
Test-FeatureFlag -PropertySetmandatory to prevent downstream null-splat binding and to fail fast with a standard missing-parameter error. - Updated user-facing documentation and changelog to reflect the new requirement and the fix.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Gatekeeper/Public/Test-FeatureFlag.ps1 | Makes -PropertySet mandatory (prevents transform crash via null splat). |
| Gatekeeper/Classes/Property.ps1 | Stops transform-time invocation of Read-PropertySet on null input. |
| docs/en-US/Test-FeatureFlag.md | Updates syntax/Required metadata for -PropertySet. |
| CHANGELOG.md | Documents the bugfix and the behavior change under Unreleased. |
Test Results 4 files 444 suites 14s ⏱️ Results for commit b260721. ♻️ This comment has been updated with latest results. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The Copilot Autofix commit (b260721) inserted [ValidateNotNull()] with a plain LF appended after the file's existing CR, producing a stray \r\r\n on that line instead of matching the file's CRLF convention.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #84.
Test-FeatureFlagforwards-PropertySetintoTest-Condition(which already requires it) via a splat:PropertySet = $PropertySet. When-PropertySetwas omitted, that forwarded$nullwas an explicit bind, which firesPropertySetTransformAttribute.Transform(). Its null branch called the argument-lessRead-PropertySet, which requires-Nameor-FilePathand can never bind — so instead of a normal missing-parameter message, callers got:(Omitting the parameter entirely never triggers
Transformat all — this only reproduces onceTest-FeatureFlagsplats the null value downstream.)Fix
Classes/Property.ps1:PropertySetTransformAttribute.Transform()now returns$nullon null input instead of calling the broken, module-function-from-a-class-methodRead-PropertySet.Public/Test-FeatureFlag.ps1:-PropertySetis now[Parameter(Mandatory)], matchingTest-Condition, so a missing value fails fast with PowerShell's standard mandatory-parameter error/prompt instead of the transform crash.docs/en-US/Test-FeatureFlag.md(Required/syntax) andCHANGELOG.md.Verification
Reproduced the exact repro from #84, confirmed error changes from the transform crash to
Cannot process command because of one or more missing mandatory parameters: PropertySet.; confirmed normal call with-PropertySetsupplied still returns the expected result. Full Pester suite: 366 passed, 0 failed, 5 skipped (pre-existing).