style: replace IsNullOrEmpty guard with ValidateNotNullOrEmpty in Split-SemVerString#7
Merged
Conversation
Syncs the same cleanup landed in PowerShellModuleTemplate PR #23 (commit abf5d46). The runtime IsNullOrEmpty check + throw on $VersionString is fail-at-bind-time material — switching to [ValidateNotNullOrEmpty()] makes the contract self-documenting and removes the redundant guard. Atomic single edit: add validator and remove guard together so the function never goes through a transient dead-code state where both exist (which is what Copilot and CodeRabbit caught on the template PR). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR aligns Split-SemVerString’s parameter validation with the pattern used elsewhere in the repo by enforcing non-null/non-empty input at parameter-binding time instead of via an explicit runtime guard.
Changes:
- Added
[ValidateNotNullOrEmpty()]toSplit-SemVerString’s$VersionStringparameter. - Removed the redundant
[string]::IsNullOrEmpty(...)+throwguard block.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 tasks
tablackburn
added a commit
that referenced
this pull request
May 12, 2026
… params (#8) Completes the cross-repo validator alignment with PowerShellModuleTemplate#23. PR #7 (2026-05-10) added the validator to `Split-SemVerString -VersionString`; this commit covers the remaining six mandatory `[string]` parameters across `Compare-SemVerPrerelease`, `Test-VersionComparison`, and `Test-VersionConstraint`. Behavior for valid inputs is unchanged. Misuse (null or empty version/prerelease strings) now fails at parameter binding instead of producing downstream NullReferenceExceptions inside the comparison logic.
3 tasks
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
Syncs the same cleanup landed in PowerShellModuleTemplate PR #23 (specifically commit
abf5d46) into JsmOperations.Split-SemVerStringintests/ManifestHelpers.psm1currently has a runtime[string]::IsNullOrEmpty($VersionString)check +throw. Switching$VersionStringto[ValidateNotNullOrEmpty()]enforces the contract at parameter-bind time, so the runtime guard becomes redundant.What changed
tests/ManifestHelpers.psm1— added[ValidateNotNullOrEmpty()]to$VersionString; removed theif ([string]::IsNullOrEmpty(...))if-block.Atomic single edit: validator added and guard removed in the same commit so the function never passes through the transient dead-code state (validator + guard both present) that Copilot and CodeRabbit flagged on the template PR.
Why this is its own PR
Out of scope for
feature/v0.1.0-mvp(#2 — release PR; merge auto-publishes to PowerShell Gallery). Style sync items shouldn't ride a release commit.Verification
$null/''), the failure mode shifts from a runtime exception inside the function to a parameter-binding error — both halt execution; the bind-time error is more idiomatic and self-documenting.Out of scope
Other consumer repos (
YouTubeMusicPS,SrrDBAutomationToolkit,ScheduledTasksManager,PlexAutomationToolkit,ReScenePS) don't havetests/ManifestHelpers.psm1at all — JsmOperations is the only consumer affected.🤖 Generated with Claude Code