Skip to content

style: enforce quoted literals, named parameters, validation; pin LF endings#23

Merged
tablackburn merged 3 commits into
mainfrom
style/quote-literals-named-params-validation
May 10, 2026
Merged

style: enforce quoted literals, named parameters, validation; pin LF endings#23
tablackburn merged 3 commits into
mainfrom
style/quote-literals-named-params-validation

Conversation

@tablackburn
Copy link
Copy Markdown
Owner

@tablackburn tablackburn commented May 6, 2026

Summary

Two concerns, both fixing first-party files in the template so newly-scaffolded modules inherit clean defaults:

  1. Style-rule enforcementinstructions/powershell.instructions.md already mandates these for in-tree code, but several files don't follow them yet:
    • Literal string parameter values single-quoted
    • Cmdlet calls use named parameters (when more than one argument is passed)
    • Every function parameter has an appropriate validator
  2. Line-ending normalization.gitattributes now pins everything to eol=lf so Windows clones with core.autocrlf=true stop emitting the LF will be replaced by CRLF warning on every commit.

No behavioural changes.

What changed

File Change
.gitattributes * text=auto eol=lf — overrides contributor's global core.autocrlf for this repo
build.ps1 [ValidateNotNullOrEmpty()] on $Task
{{ModuleName}}/Public/Get-{{Prefix}}Example.ps1 Write-Verbose literals single-quoted (kept positional — single arg)
tests/Help.tests.ps1 Positional Split-Path / Join-Path / Get-Module / Test-Path rewritten to use named -Path / -ChildPath / -Name
tests/Manifest.tests.ps1 Positional Split-Path / Join-Path / Get-Content rewritten to named; -ErrorAction Stop literal single-quoted
tests/Meta.tests.ps1 Positional Get-TextFilesList / Test-FileUnicode / Get-Content / Select-String rewritten to use the named parameters their declarations expose (-Root, -FileInfo, -Path, -Pattern)
tests/MetaFixers.psm1 [ValidateNotNull()] on FileInfo parameters; [ValidateNotNullOrEmpty()] on Root; one positional Test-FileUnicode $_ rewritten to -FileInfo $_
tests/ManifestHelpers.psm1 [ValidateNotNullOrEmpty()] on the eight mandatory string params; positional Split-SemVerString calls switched to named -VersionString; two throw "..." literals switched to single quotes
tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1 Three nested Split-Path -Parent <var> switched to Split-Path -Path <var> -Parent; Join-Path named; Get-Module named
tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1 Same as above, plus InModuleScope $name { ... } switched to InModuleScope -ModuleName $name -ScriptBlock { ... }

⚠️ Downstream propagation

GitHub template repos are one-time copies at creation — when someone clicks "Use this template," GitHub copies the files at that moment and the new repo has its own independent commit history. Subsequent updates to this template do not auto-propagate to repositories created from it.

That means every existing repo created from this template still has the same problems and needs the equivalent fix applied separately:

  • The .gitattributes eol=lf line — same one-line addition, then git add --renormalize . && git commit.
  • The style-rule fixes — only relevant to files the downstream still has untouched (most modules diverge from the template scaffolding once they grow real code).

Repos affected (non-fork, PowerShell-language repos under tablackburn):

  • JsmOperationseol=lf already applied; style fixes already merged equivalents on feature/v0.1.0-mvp
  • PlexAutomationToolkit
  • ReScenePS
  • SrrDBAutomationToolkit
  • YouTubeMusicPS
  • ScheduledTasksManager
  • ai-agent-instruction-modules
  • PoSHTinyTinyRSS

A follow-up sweep should apply the same .gitattributes change to the eight that don't have it.

Verification

  • The template is intentionally unbuildable until Initialize-Template.ps1 runs (the manifest contains literal {{GUID}}), and CI explicitly skips lint/test in the template state. So I couldn't run ./build.ps1 here.
  • Every edited file was parse-checked with [Parser]::ParseFile (raw) or [Parser]::ParseInput (placeholder files with stubs substituted) — all clean.
  • The downstream tablackburn/JsmOperations module already runs equivalent edits through Pester + PSScriptAnalyzer + 100% coverage on every commit; it serves as de facto validation that the patterns work end-to-end.

Out of scope

  • Initialize-Template.ps1 itself — also has positional calls and missing validators, but it's a one-shot init script with very different shape and risk profile. Worth a follow-up PR if you want to apply the same rules there.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Added input validation to prevent empty/null values in build parameters, improving error handling.
  • Chores

    • Standardized text file line ending handling to prevent cross-platform issues.
    • Enhanced test framework with improved path handling and stricter parameter validation.
    • Refined verbose logging formatting.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

Warning

Rate limit exceeded

@tablackburn has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 37 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 25f19f33-d00d-46a3-b614-624bbf33c62c

📥 Commits

Reviewing files that changed from the base of the PR and between b16acbd and abf5d46.

📒 Files selected for processing (10)
  • .gitattributes
  • build.ps1
  • tests/Help.tests.ps1
  • tests/Manifest.tests.ps1
  • tests/ManifestHelpers.psm1
  • tests/Meta.tests.ps1
  • tests/MetaFixers.psm1
  • tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1
  • tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1
  • {{ModuleName}}/Public/Get-{{Prefix}}Example.ps1
📝 Walkthrough

Walkthrough

This PR strengthens the codebase's build infrastructure and test robustness by enforcing LF line endings globally, adding parameter validation to PowerShell functions, refactoring path construction in test setup code, and standardizing string quoting in verbose logging.

Changes

Configuration & Build Settings

Layer / File(s) Summary
Git Attributes
.gitattributes
Text files are normalized to LF line endings (* text=auto eol=lf); generated documentation in docs/en-US/* remains marked as linguist-generated.
Build Task Validation
build.ps1
Task parameter now enforces non-null, non-empty input via [ValidateNotNullOrEmpty()] attribute.

Test Infrastructure Hardening

Layer / File(s) Summary
Parameter Validation Strengthening
tests/ManifestHelpers.psm1, tests/MetaFixers.psm1
SemVer helpers and file-processing cmdlets now enforce stricter input validation: ValidateNotNullOrEmpty() on version/path strings and ValidateNotNull() on FileInfo objects; explicit whitespace/empty checks with standardized error messages are added to core version-comparison functions.
Path Resolution Updates
tests/Help.tests.ps1, tests/Manifest.tests.ps1, tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1, tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1
Project root and manifest paths are computed using explicit -Path and Join-Path parameters; BHBuildOutput is now dynamically set based on resolved module version; path guards via Test-Path are added before enumerating child items.
Module Import/Unload Refactoring
tests/Help.tests.ps1, tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1, tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1
Module removal now explicitly uses -ErrorAction 'Ignore' to suppress errors when the module is absent; module import uses -ErrorAction 'Stop' to fail fast on import failures.
Test Setup & Method Call Refinements
tests/Meta.tests.ps1
Get-TextFilesList calls now use named -Root parameter; Test-FileUnicode invocations use explicit -FileInfo parameter binding; Get-Content/Select-String pipeline uses explicit -Path parameter for clarity.
String Quoting & Formatting
{{ModuleName}}/Public/Get-{{Prefix}}Example.ps1
Verbose logging messages changed from double-quoted to single-quoted strings for consistency.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 With LF lines so clean and straight,
And paths that validate their fate,
Our modules leap, our tests hold tight—
A PowerShell morning shining bright! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the main changes: enforcing three style rules (quoted literals, named parameters, validation) and pinning line endings to LF.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch style/quote-literals-named-params-validation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tablackburn tablackburn changed the title style: enforce quoted literals, named parameters, and parameter validation style: enforce quoted literals, named parameters, validation; pin LF endings May 6, 2026
@tablackburn tablackburn marked this pull request as ready for review May 6, 2026 22:42
Copilot AI review requested due to automatic review settings May 6, 2026 22:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the PowerShell module template to better align in-tree files with the existing PowerShell style guidance (quoted literals, named parameters, parameter validation) and to normalize repository line endings to LF to avoid CRLF churn/warnings for Windows contributors.

Changes:

  • Enforce LF line endings repo-wide via .gitattributes.
  • Add/strengthen parameter validation attributes in build/test helper functions.
  • Replace several positional cmdlet arguments with named parameters and normalize string literal quoting in template/test files.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.gitattributes Forces LF EOL normalization across the repo.
build.ps1 Adds [ValidateNotNullOrEmpty()] to $Task.
{{ModuleName}}/Public/Get-{{Prefix}}Example.ps1 Normalizes Write-Verbose string literals to single quotes.
tests/Help.tests.ps1 Converts several positional cmdlet usages to named parameters (path/module name).
tests/Manifest.tests.ps1 Uses named parameters for path/content operations; normalizes -ErrorAction quoting.
tests/Meta.tests.ps1 Uses named parameters for MetaFixers helper calls and Select-String.
tests/MetaFixers.psm1 Adds validation attributes and uses named -FileInfo in Test-FileUnicode call site.
tests/ManifestHelpers.psm1 Adds validation attributes, normalizes throws to single quotes, uses named -VersionString.
tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1 Uses named parameters for nested Split-Path, Join-Path, and Get-Module.
tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1 Same as above; also switches InModuleScope to named parameters.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/ManifestHelpers.psm1 Outdated
Comment thread tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1
Comment thread tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/ManifestHelpers.psm1`:
- Around line 39-45: The explicit runtime check using
[string]::IsNullOrEmpty($VersionString) is dead code because the parameter
already has the [ValidateNotNullOrEmpty()] attribute; remove the if-block that
throws 'VersionString cannot be empty or null' to eliminate redundancy, or if
you need to also reject whitespace-only strings replace the attribute with
[ValidateNotNullOrWhiteSpace()] and then remove the manual IsNullOrEmpty check;
reference the parameter $VersionString and the attribute
[ValidateNotNullOrEmpty()] / the existing IsNullOrEmpty check when applying the
change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d9ab530-d123-4428-9c41-4f4aee8628b8

📥 Commits

Reviewing files that changed from the base of the PR and between f0bdb02 and b16acbd.

📒 Files selected for processing (10)
  • .gitattributes
  • build.ps1
  • tests/Help.tests.ps1
  • tests/Manifest.tests.ps1
  • tests/ManifestHelpers.psm1
  • tests/Meta.tests.ps1
  • tests/MetaFixers.psm1
  • tests/Unit/Private/Invoke-{{Prefix}}Helper.tests.ps1
  • tests/Unit/Public/Get-{{Prefix}}Example.tests.ps1
  • {{ModuleName}}/Public/Get-{{Prefix}}Example.ps1

Comment thread tests/ManifestHelpers.psm1 Outdated
tablackburn and others added 3 commits May 10, 2026 02:09
…ation

The repo's own powershell.instructions.md mandates three rules for
in-tree code:

  - literal string parameter values are single-quoted
  - cmdlet calls use named parameters (when more than one arg is passed)
  - every function parameter has an appropriate validator

This commit applies those rules to the template's first-party files —
the example public/private functions, the test scaffolding, the meta
helper modules, and build.ps1. Notable changes:

  - build.ps1 gains [ValidateNotNullOrEmpty()] on $Task
  - tests/MetaFixers.psm1 + tests/ManifestHelpers.psm1 gain validators
    on every parameter that didn't already have one
  - tests/Help.tests.ps1, tests/Manifest.tests.ps1, tests/Meta.tests.ps1
    rename positional Split-Path / Join-Path / Get-Module / Get-Content
    calls to use named -Path / -ChildPath / -Name / -Pattern parameters
  - the example Get-{{Prefix}}Example.ps1 / its tests get the same
    treatment so newly-scaffolded modules start out compliant

No behavioural changes. CI on the un-initialized template still skips
build/test (the {{GUID}} placeholder is unparseable until init runs);
parse-checks confirm every edited file is syntactically valid.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The repo already stores every text file as LF in the index, but Windows
clones with global core.autocrlf=true rewrite the working tree to CRLF
on checkout. Subsequent edits made through tooling that writes LF (most
editors and AI agents do) trigger 'LF will be replaced by CRLF the next
time Git touches it' warnings on every commit.

Pinning eol=lf in .gitattributes overrides the contributor's global
autocrlf setting for this repo and silences the warning. PowerShell on
Windows handles LF fine, and the devcontainer already runs Linux.

NOTE: GitHub template repos are one-time copies at creation, so this
fix does NOT propagate to repositories already created from this
template. Existing downstream repos must apply the same .gitattributes
change separately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit added [ValidateNotNullOrEmpty()] to $VersionString,
which makes the in-body IsNullOrEmpty check unreachable under normal
parameter binding — both Copilot and CodeRabbit flagged it as dead
code. Removing the redundant guard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tablackburn tablackburn force-pushed the style/quote-literals-named-params-validation branch from b16acbd to abf5d46 Compare May 10, 2026 06:10
@tablackburn tablackburn merged commit ec17dd4 into main May 10, 2026
11 checks passed
@tablackburn tablackburn deleted the style/quote-literals-named-params-validation branch May 10, 2026 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants