From 08376a680cee07edfba8e5ccb71c7b535bc6e9da Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 24 Aug 2026 22:46:02 -0700 Subject: [PATCH 1/7] Add ShouldProcess and comment-based-help AST sweep tests Two CI tripwires over the whole Public/ population, in the style of Tests/PfbEmptyPipelineGuardCoverage.Tests.ps1: AST assertions, not behaviour tests, that catch a 544-cmdlet generated-and-hand-edited population decaying silently as new cmdlets arrive without the convention. PfbShouldProcessCoverage asserts that state-changing verbs declare SupportsShouldProcess, that a declaration is actually called, that the call is not in an inner scope, that read-only verbs never declare it, and -- the load-bearing one -- that every Remove-* declares ConfirmImpact = 'High'. ConfirmImpact prompts only when it meets or exceeds $ConfirmPreference, whose default is High, so a Remove-* at Medium deletes without ever prompting. PfbHelpCoverage asserts a non-empty .SYNOPSIS per cmdlet, a .PARAMETER entry per declared parameter matched BY NAME, no .PARAMETER naming a parameter that does not exist, and a non-empty body on each. Name matching rather than counting is what catches a rename, where the counts stay equal. Both files assert a population floor first. Every other assertion is "the offender set is empty", so an empty Public/ glob or a regressed AST walk would pass all of them while checking nothing. Both also carry a mutation-proof It that runs the record builders against synthetic fixtures with known answers in each direction, so a parser that flags nothing cannot read as green. Test-PfbNestedInInnerScope is duplicated from PfbEmptyPipelineGuardCoverage.Tests.ps1 rather than shared. Extracting one copy to tools/lib/ is the right end state and is a deliberate follow-up; refactoring a CI-critical gate was out of scope here. Remove-PfbWorkloadTag, the only Remove-* at ConfirmImpact = 'Medium', is parked in a clearly-marked PENDING-DECISION list of its own. Whether it is a defect or a deliberate choice is the maintainer's call and must be settled before merge. Pure parsing throughout -- neither file imports the module, so no module state is created or leaked. Both pass under pwsh 7 and Windows PowerShell 5.1. Co-Authored-By: Claude Opus 5 (1M context) --- Tests/PfbHelpCoverage.Tests.ps1 | 492 +++++++++++++++++++++++ Tests/PfbShouldProcessCoverage.Tests.ps1 | 475 ++++++++++++++++++++++ 2 files changed, 967 insertions(+) create mode 100644 Tests/PfbHelpCoverage.Tests.ps1 create mode 100644 Tests/PfbShouldProcessCoverage.Tests.ps1 diff --git a/Tests/PfbHelpCoverage.Tests.ps1 b/Tests/PfbHelpCoverage.Tests.ps1 new file mode 100644 index 0000000..1462a36 --- /dev/null +++ b/Tests/PfbHelpCoverage.Tests.ps1 @@ -0,0 +1,492 @@ +#Requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '5.0' } + +# Comment-based-help coverage sweep. Same rationale and same shape as +# Tests/PfbEmptyPipelineGuardCoverage.Tests.ps1 and Tests/PfbShouldProcessCoverage.Tests.ps1: +# Public/ is a 544-cmdlet generated-and-hand-edited population, and undocumented surface is exactly +# the kind of decay nothing else notices -- every other test in Tests/ is scoped to one cmdlet's +# behaviour and none of them looks at help at all. +# +# Two directions of drift matter, and only one of them is visible to a coverage count: +# - a parameter with no .PARAMETER entry (the count sees it); +# - a .PARAMETER entry naming a parameter that no longer exists (the count does NOT -- rename a +# parameter without renaming its help and the total stays put while the help is now wrong). +# Both are asserted below, by NAME MATCHING rather than by comparing counts. +# +# Deliberately NOT Get-Help. Get-Help requires importing the module, which drags module state into +# a file that is otherwise pure parsing; it also resolves help from external MAML and from the +# .NOTES blocks tools/Update-PfbContextHelp.ps1 injects, which is a different question from +# "is the source comment correct". Token-stream parsing answers the source question directly. + +BeforeAll { + $script:moduleRoot = Split-Path -Parent $PSScriptRoot + $script:publicRoot = Join-Path $script:moduleRoot 'Public' + + # Split a comment-based help block into its sections. + # + # Returns one record per help keyword, in source order, carrying the keyword, its argument + # (the parameter name, for .PARAMETER) and the body text beneath it. + # + # The keyword line must be the WHOLE trimmed line -- `^\.([A-Za-z]+)(\s+(\S+))?$`. Anchoring + # both ends is what keeps an .EXAMPLE body from being misread as a section: an example line such + # as `.\tools\Update-PfbContextHelp.ps1 -WhatIf` starts with a dot but does not match, and + # neither does prose that happens to begin a sentence with one. + function Get-PfbHelpSection { + param( + [string]$Text + ) + + # Strip the block delimiters so `<#` and `#>` cannot land inside a body and make an + # otherwise-empty section look populated. + $body = $Text -replace '^\s*<#', '' -replace '#>\s*$', '' + + $sections = [System.Collections.Generic.List[object]]::new() + $current = $null + foreach ($line in ($body -split "\r?\n")) { + if ($line.Trim() -match '^\.([A-Za-z]+)(?:\s+(\S+))?$') { + $current = [PSCustomObject]@{ + Keyword = $Matches[1].ToUpperInvariant() + Argument = $Matches[2] + BodyLines = [System.Collections.Generic.List[string]]::new() + } + $sections.Add($current) + continue + } + if ($null -ne $current) { $current.BodyLines.Add($line) } + } + return $sections + } + + # Reduce one cmdlet to the facts the assertions below need. + function Get-PfbHelpRecord { + param( + [System.Management.Automation.Language.FunctionDefinitionAst]$Function, + [System.Management.Automation.Language.Token[]]$Tokens, + [string]$File + ) + + $declared = @() + $paramBlock = $Function.Body.ParamBlock + if ($null -ne $paramBlock) { + $declared = @($paramBlock.Parameters | ForEach-Object { $_.Name.VariablePath.UserPath }) + } + + # Locate the help block from the TOKEN stream, not by regexing the file: a comment token is + # the only thing that is definitely a comment, and the AST has no node for one. + # + # A `.SYNOPSIS` keyword line is the marker (anchored, per Get-PfbHelpSection's reasoning), + # so a .DESCRIPTION or .EXAMPLE that merely mentions the word cannot be mistaken for the + # block. Prefer a block INSIDE the function extent -- the convention throughout Public/ -- + # and fall back to the nearest block above the function, which is the other legal placement + # for comment-based help. Measured on main 2026-08-24: all 543 blocks are inside, so the + # fallback is future-proofing, not a live case. + $candidates = @($Tokens | Where-Object { + $_.Kind -eq [System.Management.Automation.Language.TokenKind]::Comment -and + $_.Text -match '(?m)^\s*\.SYNOPSIS\s*$' + }) + $inside = @($candidates | Where-Object { + $_.Extent.StartOffset -ge $Function.Extent.StartOffset -and + $_.Extent.EndOffset -le $Function.Extent.EndOffset + }) + $above = @($candidates | Where-Object { + $_.Extent.EndOffset -le $Function.Extent.StartOffset + }) + + $help = $null + if ($inside.Count -gt 0) { $help = $inside[0] } + elseif ($above.Count -gt 0) { $help = $above[-1] } + + $synopsisEmpty = $false + $documented = @() + $emptyParameterSections = @() + $namelessParameterSections = 0 + + if ($null -ne $help) { + $sections = Get-PfbHelpSection -Text $help.Text + + foreach ($section in $sections) { + $sectionBody = ($section.BodyLines -join "`n").Trim() + + if ($section.Keyword -eq 'SYNOPSIS') { + # First .SYNOPSIS wins; a second one is not a shape this tree produces. + if (-not $synopsisEmpty -and [string]::IsNullOrWhiteSpace($sectionBody)) { + $synopsisEmpty = $true + } + continue + } + + if ($section.Keyword -ne 'PARAMETER') { continue } + + if ([string]::IsNullOrEmpty($section.Argument)) { + # `.PARAMETER` with no name documents nothing and names nothing, so neither the + # coverage nor the orphan assertion would see it. Counted separately. + $namelessParameterSections++ + continue + } + + $documented += $section.Argument + if ([string]::IsNullOrWhiteSpace($sectionBody)) { + $emptyParameterSections += $section.Argument + } + } + } + + # Case-insensitive both ways: PowerShell parameter names are case-insensitive, so a + # `.PARAMETER filter` documenting `[string]$Filter` is correct help and must not read as a + # miss in one direction and an orphan in the other. + $missing = @($declared | Where-Object { + $name = $_ + -not (@($documented | Where-Object { $_ -eq $name }).Count) + }) + $orphaned = @($documented | Where-Object { + $name = $_ + -not (@($declared | Where-Object { $_ -eq $name }).Count) + }) + $duplicated = @($documented | Group-Object | Where-Object { $_.Count -gt 1 } | + ForEach-Object { $_.Name }) + + [PSCustomObject]@{ + File = $File + Function = $Function.Name + Line = $Function.Extent.StartLineNumber + HasHelpBlock = ($null -ne $help) + SynopsisEmpty = $synopsisEmpty + DeclaredParameters = $declared + DocumentedParameters = $documented + MissingParameters = $missing + OrphanedParameters = $orphaned + DuplicatedParameters = $duplicated + EmptyParameterSections = $emptyParameterSections + NamelessParameterSections = $namelessParameterSections + } + } + + # One record per cmdlet. FindAll(..., $false) takes only DEPTH-0 function definitions so a + # nested helper is never mistaken for a cmdlet, and the first of those is the cmdlet -- matching + # the file-per-cmdlet layout Public/ uses throughout. + $script:cmdlets = @( + foreach ($file in (Get-ChildItem -Path $script:publicRoot -Filter '*.ps1' -Recurse -File)) { + $relative = $file.FullName.Substring($script:moduleRoot.Length).TrimStart('\', '/').Replace('\', '/') + + $tokens = $null + $errors = $null + $ast = [System.Management.Automation.Language.Parser]::ParseFile( + $file.FullName, [ref]$tokens, [ref]$errors) + if ($errors.Count -gt 0) { + throw "Parse errors in ${relative}: $(($errors | ForEach-Object { $_.Message }) -join '; ')" + } + + $functions = @($ast.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] + }, $false)) + if ($functions.Count -eq 0) { continue } + + Get-PfbHelpRecord -Function $functions[0] -Tokens $tokens -File $relative + } + ) + + $script:withParameters = @($script:cmdlets | Where-Object { $_.DeclaredParameters.Count -gt 0 }) + $script:totalDeclared = (@($script:cmdlets | ForEach-Object { $_.DeclaredParameters.Count }) | + Measure-Object -Sum).Sum + $script:totalDocumented = (@($script:cmdlets | ForEach-Object { $_.DocumentedParameters.Count }) | + Measure-Object -Sum).Sum +} + +Describe 'Comment-based help coverage' { + + It 'scans a population large enough for the other assertions to mean something' { + # THE ANTI-VACUOUS FLOOR. Every assertion below is "the set of offenders is empty". If the + # Public/ glob returned nothing, or the depth-0 function walk regressed, all of them would + # pass having examined zero cmdlets and the gate would report green while checking nothing. + # + # Floors, not pins. The population grows -- pinning 544 turns every legitimate new cmdlet + # into an unrelated red build, which is how a gate ends up disabled. Measured on main + # 2026-08-24: 544 cmdlets, 542 with parameters, 2860 declared parameters, 2845 documented + # (2860 after the six help fixes that land with this file). + # + # The derived floors carry as much weight as the total. A total-only floor survives intact + # while a regression in the token-stream help lookup drives every DocumentedParameters to + # empty -- which would empty the orphan assertion (nothing documented, nothing to orphan) + # while making the coverage assertion red for the right reason. Flooring the documented + # total makes that failure explicit instead of half-visible. + $script:cmdlets.Count | Should -BeGreaterOrEqual 500 + $script:withParameters.Count | + Should -BeGreaterOrEqual 500 -Because 'the parameter assertions are scoped to cmdlets that declare parameters' + $script:totalDeclared | + Should -BeGreaterOrEqual 2500 -Because 'a regression in the ParamBlock walk empties the coverage assertion without emptying the cmdlet count' + $script:totalDocumented | + Should -BeGreaterOrEqual 2500 -Because 'a regression in the help-block lookup empties the orphan assertion without emptying the cmdlet count' + } + + It 'gives every cmdlet a .SYNOPSIS with text in it' { + # Presence and content in one assertion, because they fail together in practice: a block + # that lost its text is the same defect as a block that was never written, and splitting + # them would let a bare `.SYNOPSIS` line satisfy a presence check while documenting nothing. + $missingBlock = @($script:cmdlets | Where-Object { -not $_.HasHelpBlock }) + $missingDetail = @($missingBlock | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" + $missingDetail | Should -BeNullOrEmpty -Because "every public cmdlet needs comment-based help with a .SYNOPSIS; offenders:`n$missingDetail" + + $emptySynopsis = @($script:cmdlets | Where-Object { $_.SynopsisEmpty }) + $emptyDetail = @($emptySynopsis | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" + $emptyDetail | Should -BeNullOrEmpty -Because "a .SYNOPSIS with no text satisfies a presence check while documenting nothing; offenders:`n$emptyDetail" + } + + It 'documents every declared parameter, matching by name' { + # By NAME, never by count. A count comparison passes when a file has the right number of + # .PARAMETER entries naming the wrong parameters -- which is exactly what a rename produces. + $offenders = @($script:withParameters | Where-Object { $_.MissingParameters.Count -gt 0 }) + $detail = @($offenders | ForEach-Object { + "$($_.File): $($_.Function) -- undocumented: $($_.MissingParameters -join ', ')" + }) -join "`n" + $detail | Should -BeNullOrEmpty -Because "every declared parameter needs a .PARAMETER entry; offenders:`n$detail" + } + + It 'names no parameter that does not exist' { + # The drift direction a coverage count cannot see. Rename or remove a parameter without + # touching its help and the documented total is unchanged, so coverage still reads 100% + # while Get-Help now describes a parameter the cmdlet does not have. + $orphans = @($script:cmdlets | Where-Object { $_.OrphanedParameters.Count -gt 0 }) + $orphanDetail = @($orphans | ForEach-Object { + "$($_.File): $($_.Function) -- documents nonexistent: $($_.OrphanedParameters -join ', ')" + }) -join "`n" + $orphanDetail | Should -BeNullOrEmpty -Because "a .PARAMETER entry naming a parameter that does not exist is help describing a cmdlet that no longer exists; offenders:`n$orphanDetail" + + # Two adjacent malformations, both invisible to the assertions above. A duplicate entry + # means one of the two is stale (or a copy-paste that should have been renamed); a bare + # `.PARAMETER` with no name documents nothing and orphans nothing. Both measured 0 on main. + $duplicates = @($script:cmdlets | Where-Object { $_.DuplicatedParameters.Count -gt 0 }) + $duplicateDetail = @($duplicates | ForEach-Object { + "$($_.File): $($_.Function) -- duplicated: $($_.DuplicatedParameters -join ', ')" + }) -join "`n" + $duplicateDetail | Should -BeNullOrEmpty -Because "a duplicated .PARAMETER entry means one of the two is stale; offenders:`n$duplicateDetail" + + $nameless = @($script:cmdlets | Where-Object { $_.NamelessParameterSections -gt 0 }) + $namelessDetail = @($nameless | ForEach-Object { + "$($_.File): $($_.Function) -- $($_.NamelessParameterSections) nameless .PARAMETER" + }) -join "`n" + $namelessDetail | Should -BeNullOrEmpty -Because "a .PARAMETER with no name documents nothing and is invisible to both coverage and orphan checks; offenders:`n$namelessDetail" + } + + It 'gives every .PARAMETER entry a body' { + # A bare `.PARAMETER Name` followed immediately by the next keyword satisfies the + # name-matching assertion above while telling the reader nothing. Split out from that + # assertion because the remedy is different: the entry exists and needs prose, rather than + # being absent and needing to be added. + $offenders = @($script:cmdlets | Where-Object { $_.EmptyParameterSections.Count -gt 0 }) + $detail = @($offenders | ForEach-Object { + "$($_.File): $($_.Function) -- empty: $($_.EmptyParameterSections -join ', ')" + }) -join "`n" + $detail | Should -BeNullOrEmpty -Because "a .PARAMETER entry with no text documents nothing; offenders:`n$detail" + } + + It 'recognises every shape it is meant to flag, and none it is not' { + # The mutation proof. Every assertion above is "offenders is empty", and on main every one + # of those sets IS empty -- so a parser bug that returned no sections at all, or that + # matched nothing, would leave them empty for the wrong reason and the whole file would be + # green while inert. These fixtures give each predicate a known answer in each direction. + # + # The last two are the false-positive guard: an .EXAMPLE body containing a dot-leading line + # and a .DESCRIPTION containing the word SYNOPSIS must not be misread as section keywords. + $fixtures = [ordered]@{ + 'clean' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Retrieves a fixture. + .PARAMETER Name + The fixture name. + .PARAMETER Array + The connection. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name, [Parameter()] [PSCustomObject]$Array) +} +'@ + 'no-help' = @' +function Get-PfbFixture { + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'empty-synopsis' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'undocumented' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Retrieves a fixture. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name, [Parameter()] [PSCustomObject]$Array) +} +'@ + 'orphaned' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Retrieves a fixture. + .PARAMETER Name + The fixture name. + .PARAMETER Removed + A parameter that was deleted from param() without deleting its help. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'renamed' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Retrieves a fixture. + .PARAMETER OldName + Renamed in param() but not here -- the documented COUNT is still 1 of 1. + #> + [CmdletBinding()] + param([Parameter()] [string]$NewName) +} +'@ + 'empty-parameter' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Retrieves a fixture. + .PARAMETER Name + .EXAMPLE + Get-PfbFixture + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'duplicate' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Retrieves a fixture. + .PARAMETER Name + First entry. + .PARAMETER Name + Second, stale entry. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'nameless' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Retrieves a fixture. + .PARAMETER + No name on the keyword line. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'dotted-prose' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Retrieves a fixture. The word SYNOPSIS appears here and must not start a section. + .DESCRIPTION + Regenerate with the tool below. + .PARAMETER Name + The fixture name. + .EXAMPLE + .\tools\Update-PfbContextHelp.ps1 -WhatIf + + A dot-leading line inside an example body. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'case-mismatch' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Retrieves a fixture. + .PARAMETER name + Lower-case name for an upper-case parameter -- correct help, not a defect. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + } + + $records = [ordered]@{} + foreach ($label in $fixtures.Keys) { + $tokens = $null + $errors = $null + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + $fixtures[$label], [ref]$tokens, [ref]$errors) + if ($errors.Count -gt 0) { + throw "Parse errors in ${label}: $(($errors | ForEach-Object { $_.Message }) -join '; ')" + } + $fn = $ast.Find({ + param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] + }, $true) + $records[$label] = Get-PfbHelpRecord -Function $fn -Tokens $tokens -File $label + } + + # A clean cmdlet must produce NO findings at all. Without this every other expectation + # below could be satisfied by a parser that flags everything. + $records['clean'].HasHelpBlock | Should -BeTrue + $records['clean'].SynopsisEmpty | Should -BeFalse + $records['clean'].DocumentedParameters.Count | Should -Be 2 + $records['clean'].MissingParameters | Should -BeNullOrEmpty + $records['clean'].OrphanedParameters | Should -BeNullOrEmpty + $records['clean'].DuplicatedParameters | Should -BeNullOrEmpty + $records['clean'].EmptyParameterSections | Should -BeNullOrEmpty + $records['clean'].NamelessParameterSections | Should -Be 0 + + $records['no-help'].HasHelpBlock | Should -BeFalse + $records['empty-synopsis'].HasHelpBlock | Should -BeTrue + $records['empty-synopsis'].SynopsisEmpty | Should -BeTrue + + $records['undocumented'].MissingParameters | Should -Be @('Array') + + $records['orphaned'].OrphanedParameters | Should -Be @('Removed') + $records['orphaned'].MissingParameters | Should -BeNullOrEmpty + + # The rename case is the whole reason assertion 3 exists: counts match exactly (1 declared, + # 1 documented) and only name matching sees the drift. + $records['renamed'].DeclaredParameters.Count | Should -Be $records['renamed'].DocumentedParameters.Count + $records['renamed'].MissingParameters | Should -Be @('NewName') + $records['renamed'].OrphanedParameters | Should -Be @('OldName') + + $records['empty-parameter'].EmptyParameterSections | Should -Be @('Name') + $records['empty-parameter'].MissingParameters | + Should -BeNullOrEmpty -Because 'the entry exists, so it is an empty-body finding and not a coverage finding' + + $records['duplicate'].DuplicatedParameters | Should -Be @('Name') + $records['nameless'].NamelessParameterSections | Should -Be 1 + $records['nameless'].MissingParameters | + Should -Be @('Name') -Because 'a nameless entry documents nothing, so the parameter is still undocumented' + + # False-positive guards. + $records['dotted-prose'].MissingParameters | Should -BeNullOrEmpty + $records['dotted-prose'].OrphanedParameters | Should -BeNullOrEmpty + $records['dotted-prose'].EmptyParameterSections | Should -BeNullOrEmpty + $records['dotted-prose'].SynopsisEmpty | Should -BeFalse + + $records['case-mismatch'].MissingParameters | + Should -BeNullOrEmpty -Because 'PowerShell parameter names are case-insensitive, so .PARAMETER name documents $Name' + $records['case-mismatch'].OrphanedParameters | Should -BeNullOrEmpty + } +} diff --git a/Tests/PfbShouldProcessCoverage.Tests.ps1 b/Tests/PfbShouldProcessCoverage.Tests.ps1 new file mode 100644 index 0000000..194c702 --- /dev/null +++ b/Tests/PfbShouldProcessCoverage.Tests.ps1 @@ -0,0 +1,475 @@ +#Requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '5.0' } + +# ShouldProcess coverage sweep. Public/ is a 544-cmdlet generated-and-hand-edited population and a +# population that size decays silently: a new state-changing cmdlet arrives without +# SupportsShouldProcess, or with a declaration nothing ever calls, and no existing test notices +# because every test in Tests/ is scoped to one cmdlet. +# +# These are AST tripwires, not behaviour tests -- same shape as +# Tests/PfbEmptyPipelineGuardCoverage.Tests.ps1, and for the same reason: a mechanical gate runs on +# every change for free and cannot rationalise a finding away. +# +# The load-bearing assertion is the ConfirmImpact one. ConfirmImpact only triggers an automatic +# prompt when it meets or exceeds $ConfirmPreference, which defaults to 'High'. A Remove-* cmdlet +# declaring 'Medium' therefore deletes without ever prompting -- the declaration reads as +# protective while doing nothing at all. +# +# Nothing here imports the module. Parsing only, so no module state is created or leaked. + +BeforeAll { + $script:moduleRoot = Split-Path -Parent $PSScriptRoot + $script:publicRoot = Join-Path $script:moduleRoot 'Public' + + # DUPLICATED, deliberately, from Tests/PfbEmptyPipelineGuardCoverage.Tests.ps1. Extracting a + # single shared copy to tools/lib/ is the right end state and is a deliberate follow-up, not an + # oversight: that file is a CI-critical gate and refactoring it was out of scope for the change + # that added this one. If you edit one copy, edit both. + # + # Walk a node's parent chain up to (and excluding) $Stop, reporting whether any INNER SCOPE sits + # in between. Two AST shapes introduce one: a ScriptBlockExpressionAst (the familiar + # `... | ForEach-Object { }` case) and a nested FunctionDefinitionAst. A NamedBlockAst + # (begin/process/end) and a StatementBlockAst (an if body, a foreach body) are NEITHER -- they + # share the cmdlet's scope, so a `return` in one returns from the cmdlet. + # + # Matching a FunctionDefinitionAst cannot flag a cmdlet's own definition: the only call site + # stops at $Function itself and the loop tests its condition before its body, so the cmdlet's + # own FunctionDefinitionAst is never reached. + function Test-PfbNestedInInnerScope { + param( + [System.Management.Automation.Language.Ast]$Node, + [System.Management.Automation.Language.Ast]$Stop + ) + + $cursor = $Node.Parent + while ($null -ne $cursor -and -not [object]::ReferenceEquals($cursor, $Stop)) { + if ($cursor -is [System.Management.Automation.Language.ScriptBlockExpressionAst] -or + $cursor -is [System.Management.Automation.Language.FunctionDefinitionAst]) { + return $true + } + $cursor = $cursor.Parent + } + return $false + } + + # The [CmdletBinding(...)] attribute of a function, or $null. + # + # It hangs off the PARAM BLOCK, not off the FunctionDefinitionAst -- a function with no param() + # block cannot carry one at all, which is why this returns $null rather than throwing. + function Get-PfbCmdletBindingAttribute { + param( + [System.Management.Automation.Language.FunctionDefinitionAst]$Function + ) + + $paramBlock = $Function.Body.ParamBlock + if ($null -eq $paramBlock) { return $null } + + foreach ($attribute in $paramBlock.Attributes) { + if ($attribute -isnot [System.Management.Automation.Language.AttributeAst]) { continue } + # Both spellings are legal PowerShell; the shipped tree uses the short one throughout, + # but accepting only the short one would make a legal hand edit invisible to this gate. + if ($attribute.TypeName.Name -in @('CmdletBinding', 'CmdletBindingAttribute')) { + return $attribute + } + } + return $null + } + + # Reduce one cmdlet to the facts the assertions below need. + function Get-PfbShouldProcessRecord { + param( + [System.Management.Automation.Language.FunctionDefinitionAst]$Function, + [string]$File + ) + + $supportsShouldProcess = $false + $confirmImpact = $null + + $binding = Get-PfbCmdletBindingAttribute -Function $Function + if ($null -ne $binding) { + foreach ($named in $binding.NamedArguments) { + if ($named.ArgumentName -eq 'SupportsShouldProcess') { + # `SupportsShouldProcess` with no `= $true` omits the expression entirely; that + # is the form the whole tree uses, so treating ExpressionOmitted as $false would + # read the population as having zero declarations and pass everything vacuously. + if ($named.ExpressionOmitted) { $supportsShouldProcess = $true } + elseif ($named.Argument.Extent.Text -eq '$true') { $supportsShouldProcess = $true } + } + elseif ($named.ArgumentName -eq 'ConfirmImpact') { + if (-not $named.ExpressionOmitted -and + $named.Argument -is [System.Management.Automation.Language.StringConstantExpressionAst]) { + $confirmImpact = $named.Argument.Value + } + else { + # A non-literal ConfirmImpact (a variable, an expression) is not something + # this gate can evaluate. Record it verbatim so the assertion reds and a + # human looks, rather than silently reading as "not High". + $confirmImpact = $named.Argument.Extent.Text + } + } + } + } + + # $PSCmdlet.ShouldProcess(...) / .ShouldContinue(...). Member is an ExpressionAst, so a + # dynamic member name ($PSCmdlet.$verb(...)) is a MemberExpression rather than a string + # constant -- guard the cast instead of stringifying, or a dynamic call would compare equal + # to nothing and be silently uncounted. + $calls = @($Function.FindAll({ + param($node) + if ($node -isnot [System.Management.Automation.Language.InvokeMemberExpressionAst]) { return $false } + if ($node.Member -isnot [System.Management.Automation.Language.StringConstantExpressionAst]) { return $false } + return ($node.Member.Value -in @('ShouldProcess', 'ShouldContinue')) + }, $true)) + + $nestedCalls = @($calls | Where-Object { + Test-PfbNestedInInnerScope -Node $_ -Stop $Function + }) + + [PSCustomObject]@{ + File = $File + Function = $Function.Name + Verb = ($Function.Name -split '-', 2)[0] + Line = $Function.Extent.StartLineNumber + SupportsShouldProcess = $supportsShouldProcess + ConfirmImpact = $confirmImpact + ShouldProcessCalls = $calls.Count + NestedCalls = $nestedCalls.Count + NestedCallLines = @($nestedCalls | ForEach-Object { $_.Extent.StartLineNumber }) + } + } + + # One record per cmdlet. FindAll(..., $false) takes only DEPTH-0 function definitions, so a + # nested helper function inside a cmdlet is never mistaken for a cmdlet of its own -- and the + # first of those is the cmdlet, matching the file-per-cmdlet layout Public/ uses throughout. + $script:cmdlets = @( + foreach ($file in (Get-ChildItem -Path $script:publicRoot -Filter '*.ps1' -Recurse -File)) { + $relative = $file.FullName.Substring($script:moduleRoot.Length).TrimStart('\', '/').Replace('\', '/') + + $tokens = $null + $errors = $null + $ast = [System.Management.Automation.Language.Parser]::ParseFile( + $file.FullName, [ref]$tokens, [ref]$errors) + if ($errors.Count -gt 0) { + throw "Parse errors in ${relative}: $(($errors | ForEach-Object { $_.Message }) -join '; ')" + } + + $functions = @($ast.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] + }, $false)) + if ($functions.Count -eq 0) { continue } + + Get-PfbShouldProcessRecord -Function $functions[0] -File $relative + } + ) + + # The verbs that change ARRAY state. Deliberately a small explicit list rather than + # "everything that is not Get/Test": a new verb should have to be considered here rather than + # silently inheriting a requirement that may not fit it. + $script:stateChangingVerbs = @('New', 'Remove', 'Update', 'Set', 'Add', 'Clear') + + # The read-only verbs. Declaring SupportsShouldProcess on one of these advertises a mutation + # that does not exist -- -WhatIf output claiming a Get- cmdlet would change something. + $script:readOnlyVerbs = @('Get', 'Test') + + # A LITERAL list, never a name-shaped regex. A pattern like '*Context*' or '*Credential*' would + # silently absorb a future cmdlet that genuinely does need a guard, which is precisely the decay + # this file exists to catch. + # + # Every entry here changes only LOCAL SESSION state or is a read-only diagnostic. None of them + # sends a mutating request, so -WhatIf/-Confirm would be noise rather than protection. + $script:shouldProcessExempt = @( + 'Set-PfbCredential' # local credential store; no array call + 'Clear-PfbCredential' # local credential store; no array call + 'Set-PfbContext' # local context state on a copied connection object; no array call + 'Clear-PfbContext' # local context state on a copied connection object; no array call + + # The three below carry the Invoke verb, which is NOT in $script:stateChangingVerbs, so + # they are inert today. Listed anyway so that the reasoning survives: if Invoke is ever + # added to the verb list, these three must not become failures by accident. + 'Invoke-PfbInContext' # scoping wrapper; mutates nothing itself, the wrapped call does + 'Invoke-PfbNetworkPing' # read-only diagnostic + 'Invoke-PfbNetworkTrace' # read-only diagnostic + ) + + # PENDING MAINTAINER DECISION -- NOT a settled exemption. + # + # Remove-PfbWorkloadTag is the only Remove-* in the module declaring ConfirmImpact = 'Medium' + # rather than 'High'. At Medium it deletes without ever prompting, because the default + # $ConfirmPreference is High. That is either: + # (a) a real defect -- the cmdlet should be 'High' like its 111 siblings; or + # (b) deliberate -- a workload TAG is metadata, cheaply recreated, and unlike the other + # Remove-* cmdlets its loss destroys no data. + # + # Which of those is true is the maintainer's call, not this test's, and it MUST be settled + # before this file merges. It is parked here, in its own list with its own name, rather than + # buried in $script:confirmImpactExempt, so that resolving it is a visible one-line edit: + # either move it into the settled list with a reason, or fix the cmdlet and delete this list. + $script:confirmImpactPendingDecision = @( + 'Remove-PfbWorkloadTag' + ) + + # Settled exemptions from the Remove-*-is-High rule. Empty today, and it should stay that way + # unless a specific cmdlet earns an entry with a written reason. + $script:confirmImpactExempt = @() +} + +Describe 'ShouldProcess coverage' { + + It 'scans a population large enough for the other assertions to mean something' { + # THE ANTI-VACUOUS FLOOR. Every assertion below is of the form "the set of offenders is + # empty". If the Public/ glob returned nothing, or the depth-0 function walk regressed, + # every one of them would pass while checking exactly zero cmdlets and the gate would + # report green. These floors are the only thing standing between that and a false pass. + # + # Floors, not pins. The population grows -- pinning 544 turns every legitimate new cmdlet + # into an unrelated red build, which is how a gate gets disabled. Measured on main + # 2026-08-24: 544 cmdlets, 311 declaring SupportsShouldProcess, 112 Remove-*. + # + # The derived floors matter as much as the total: a total-only floor passes intact while a + # regression in the CmdletBinding walk drives every SupportsShouldProcess to $false, which + # would empty the two assertions that depend on it. + $script:cmdlets.Count | Should -BeGreaterOrEqual 500 + + $declaring = @($script:cmdlets | Where-Object SupportsShouldProcess) + $declaring.Count | + Should -BeGreaterOrEqual 280 -Because 'a regression in the CmdletBinding/NamedArguments walk empties the "declaration is used" and "impact is High" assertions without emptying the total' + + $removes = @($script:cmdlets | Where-Object { $_.Verb -eq 'Remove' }) + $removes.Count | + Should -BeGreaterOrEqual 100 -Because 'the ConfirmImpact assertion is scoped to Remove-*, so it needs its own floor' + } + + It 'declares SupportsShouldProcess on every state-changing cmdlet' { + $offenders = @($script:cmdlets | Where-Object { + $_.Verb -in $script:stateChangingVerbs -and + -not $_.SupportsShouldProcess -and + $_.Function -notin $script:shouldProcessExempt + }) + $detail = @($offenders | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" + $detail | Should -BeNullOrEmpty -Because "a state-changing cmdlet without SupportsShouldProcess silently ignores -WhatIf and -Confirm; offenders:`n$detail" + } + + It 'actually calls ShouldProcess wherever it declares SupportsShouldProcess' { + # A declaration with no call is worse than no declaration: -WhatIf binds successfully, the + # caller believes nothing happened, and the request went out anyway. Measured 0 violations + # on main -- this is a tripwire protecting a clean state, not a fix for a live defect. + $offenders = @($script:cmdlets | Where-Object { + $_.SupportsShouldProcess -and $_.ShouldProcessCalls -eq 0 + }) + $detail = @($offenders | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" + $detail | Should -BeNullOrEmpty -Because "SupportsShouldProcess without a ShouldProcess call makes -WhatIf silently perform the operation; offenders:`n$detail" + } + + It 'keeps every ShouldProcess call out of an inner scope' { + # Same hazard the empty-pipeline sweep documents. The shipped shape is + # `if ($PSCmdlet.ShouldProcess(...)) { Invoke-PfbApiRequest ... }` as a direct statement of + # the cmdlet block. Inside a ForEach-Object scriptblock or a nested function the guard's + # control flow applies to THAT scope, so `return`-style declines leak and the request still + # goes out. + # + # This flags ANY nested call, not merely "no unnested call exists". That is stricter than + # the harm strictly requires -- a genuine per-item confirmation loop written as + # `$items | ForEach-Object { if ($PSCmdlet.ShouldProcess($_)) { ... } }` is sound and would + # be flagged. The module has no such cmdlet today (measured 0), the convention here is a + # single top-level guard, and the allowlist below is the intended escape valve if one is + # ever written deliberately. Explicit and empty on purpose. + $allowedNestedShouldProcess = @() + + $offenders = @($script:cmdlets | Where-Object { + $_.NestedCalls -gt 0 -and $_.Function -notin $allowedNestedShouldProcess + }) + $detail = @($offenders | ForEach-Object { + "$($_.File):$($_.NestedCallLines -join ',') $($_.Function)" + }) -join "`n" + $detail | Should -BeNullOrEmpty -Because "a ShouldProcess call inside a scriptblock or nested function guards that scope, not the cmdlet; offenders:`n$detail" + } + + It 'declares no SupportsShouldProcess on a read-only verb' { + # The inverse tripwire. -WhatIf on a Get- cmdlet claiming it would change something is a + # lie about the cmdlet's behaviour, and it is the shape a copy-pasted CmdletBinding line + # produces. Measured 0 violations on main. + $readOnly = @($script:cmdlets | Where-Object { $_.Verb -in $script:readOnlyVerbs }) + $readOnly.Count | + Should -BeGreaterOrEqual 200 -Because 'the read-only population must be non-trivial or this assertion checks nothing' + + $offenders = @($readOnly | Where-Object { $_.SupportsShouldProcess }) + $detail = @($offenders | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" + $detail | Should -BeNullOrEmpty -Because "a read-only verb declaring SupportsShouldProcess advertises a mutation it does not perform; offenders:`n$detail" + } + + It 'declares ConfirmImpact = High on every Remove- cmdlet' { + # THE LOAD-BEARING ONE. ConfirmImpact prompts automatically only when it meets or exceeds + # $ConfirmPreference, whose default is 'High'. A Remove-* at 'Medium' or 'Low' therefore + # never prompts: the declaration reads as protective in review while doing nothing at + # runtime. The failure is invisible in mocked tests (a mock never reaches ShouldProcess) and + # invisible interactively for the cmdlets that DO prompt, so a sweep is the only place it + # can be caught. + # + # $script:confirmImpactPendingDecision is NOT a settled exemption -- see its definition. + $offenders = @($script:cmdlets | Where-Object { + $_.Verb -eq 'Remove' -and + $_.ConfirmImpact -ne 'High' -and + $_.Function -notin $script:confirmImpactExempt -and + $_.Function -notin $script:confirmImpactPendingDecision + }) + $detail = @($offenders | ForEach-Object { + "$($_.File): $($_.Function) [ConfirmImpact = $($_.ConfirmImpact)]" + }) -join "`n" + $detail | Should -BeNullOrEmpty -Because "a Remove- cmdlet below ConfirmImpact 'High' deletes without ever prompting, because `$ConfirmPreference defaults to High; offenders:`n$detail" + + # Keep the pending-decision list honest in both directions. If somebody resolves + # Remove-PfbWorkloadTag by raising it to High but forgets to empty this list, the entry + # becomes a silent standing exemption for a cmdlet that no longer needs one -- and the next + # Remove-* to regress to Medium under that same name would pass. So assert every parked + # name is still genuinely non-High. + foreach ($name in $script:confirmImpactPendingDecision) { + $record = @($script:cmdlets | Where-Object { $_.Function -eq $name }) + $record.Count | Should -Be 1 -Because "the pending-decision entry '$name' must still name a real cmdlet" + $record[0].ConfirmImpact | + Should -Not -Be 'High' -Because "'$name' is now High, so its pending-decision entry is stale and must be deleted" + } + } + + It 'does not flag a deliberate ConfirmImpact escalation outside Remove-' { + # Spec requirement, asserted rather than merely commented. Escalating a non-destructive but + # dangerous cmdlet to 'High' is correct and must never be treated as a finding. Asserting it + # positively is what stops the assertion above from later being "tidied" into + # "everything declaring SupportsShouldProcess must be High", which would red these two and + # invite the wrong fix. + $escalated = @($script:cmdlets | Where-Object { + $_.Verb -ne 'Remove' -and $_.ConfirmImpact -eq 'High' + }) + $escalated.Count | Should -BeGreaterThan 0 + + foreach ($name in @('New-PfbArrayFactoryResetToken', 'New-PfbRapidDataLockingRotation')) { + @($escalated | Where-Object { $_.Function -eq $name }).Count | + Should -Be 1 -Because "'$name' is a deliberate non-Remove escalation to High and must remain unflagged" + } + } + + It 'has a working inner-scope-nesting detector' { + # The one one-way predicate in this file, and the same protection the empty-pipeline sweep + # gives its copy. Every other detector here fails safe -- a broken CmdletBinding walk drives + # SupportsShouldProcess to $false and reds the floor, a broken call finder reds the + # "declaration is used" assertion. But if Test-PfbNestedInInnerScope regressed to always + # returning $false, the inner-scope assertion would pass vacuously and no floor would + # notice, because its offender set is empty on main either way. + # + # So assert the predicate against a fixture with a known answer in each direction, and + # assert the record-building path end-to-end on the same fixture. + $fixture = @' +function Remove-PfbFixture { + [CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'High')] + param([Parameter()] [string]$Name) + if ($PSCmdlet.ShouldProcess('target', 'direct')) { $null = 1 } + 1..2 | ForEach-Object { $PSCmdlet.ShouldProcess('target', 'in-scriptblock') } + function Invoke-Inner { + $PSCmdlet.ShouldContinue('target', 'in-nested-function') + } +} +'@ + $fixtureAst = [System.Management.Automation.Language.Parser]::ParseInput( + $fixture, [ref]$null, [ref]$null) + $fixtureFunction = $fixtureAst.Find({ + param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] + }, $true) + + $calls = @($fixtureFunction.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.InvokeMemberExpressionAst] -and + $node.Member -is [System.Management.Automation.Language.StringConstantExpressionAst] -and + $node.Member.Value -in @('ShouldProcess', 'ShouldContinue') + }, $true)) + $calls.Count | Should -Be 3 -Because 'ShouldContinue must be recognised alongside ShouldProcess' + + $answers = @($calls | ForEach-Object { + Test-PfbNestedInInnerScope -Node $_ -Stop $fixtureFunction + }) + $answers[0] | Should -BeFalse -Because 'the first call is a direct statement of the cmdlet block' + $answers[1] | Should -BeTrue -Because 'the second call is inside a ForEach-Object scriptblock' + $answers[2] | Should -BeTrue -Because 'the third call is inside a nested function definition' + + $record = Get-PfbShouldProcessRecord -Function $fixtureFunction -File 'fixture' + $record.SupportsShouldProcess | + Should -BeTrue -Because 'the attribute walk must read the expression-omitted form the whole tree uses' + $record.ConfirmImpact | Should -Be 'High' + $record.ShouldProcessCalls | Should -Be 3 + $record.NestedCalls | Should -Be 2 + $record.Verb | Should -Be 'Remove' + } + + It 'recognises the negative shapes it is meant to flag' { + # The mirror of the It above: prove each assertion's PREDICATE fires on a cmdlet that has + # the defect, not just that no cmdlet in Public/ has it. Without this, an extraction bug + # that made SupportsShouldProcess never $true, or ConfirmImpact always 'High', would leave + # every offender set empty and every assertion green. + $negatives = [ordered]@{ + 'no-shouldprocess-declaration' = @' +function Remove-PfbFixture { + [CmdletBinding()] + param([Parameter()] [string]$Name) + Invoke-PfbApiRequest -Method DELETE -Endpoint 'x' +} +'@ + 'declared-never-called' = @' +function Remove-PfbFixture { + [CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'High')] + param([Parameter()] [string]$Name) + Invoke-PfbApiRequest -Method DELETE -Endpoint 'x' +} +'@ + 'impact-medium' = @' +function Remove-PfbFixture { + [CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'Medium')] + param([Parameter()] [string]$Name) + if ($PSCmdlet.ShouldProcess('x')) { Invoke-PfbApiRequest -Method DELETE -Endpoint 'x' } +} +'@ + 'impact-absent' = @' +function Remove-PfbFixture { + [CmdletBinding(SupportsShouldProcess)] + param([Parameter()] [string]$Name) + if ($PSCmdlet.ShouldProcess('x')) { Invoke-PfbApiRequest -Method DELETE -Endpoint 'x' } +} +'@ + 'readonly-declares' = @' +function Get-PfbFixture { + [CmdletBinding(SupportsShouldProcess)] + param([Parameter()] [string]$Name) + if ($PSCmdlet.ShouldProcess('x')) { Invoke-PfbApiRequest -Method GET -Endpoint 'x' } +} +'@ + } + + $records = [ordered]@{} + foreach ($label in $negatives.Keys) { + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + $negatives[$label], [ref]$null, [ref]$null) + $fn = $ast.Find({ + param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] + }, $true) + $records[$label] = Get-PfbShouldProcessRecord -Function $fn -File $label + } + + # Assertion 1's predicate. + $records['no-shouldprocess-declaration'].SupportsShouldProcess | Should -BeFalse + $records['no-shouldprocess-declaration'].Verb | Should -BeIn $script:stateChangingVerbs + + # Assertion 2's predicate: declared, zero calls. + $records['declared-never-called'].SupportsShouldProcess | Should -BeTrue + $records['declared-never-called'].ShouldProcessCalls | Should -Be 0 + + # Assertion 5's predicate, in both the wrong-value and the omitted-value shapes. The + # omitted shape matters on its own: a missing ConfirmImpact defaults to Medium at runtime, + # so `$null -ne 'High'` has to count as an offender. + $records['impact-medium'].ConfirmImpact | Should -Be 'Medium' + $records['impact-absent'].ConfirmImpact | Should -BeNullOrEmpty + $records['impact-absent'].ConfirmImpact | Should -Not -Be 'High' + + # Assertion 4's predicate. + $records['readonly-declares'].Verb | Should -BeIn $script:readOnlyVerbs + $records['readonly-declares'].SupportsShouldProcess | Should -BeTrue + } +} From 5bce445c13f98bf107908d95770894cd27bc2522 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 24 Aug 2026 22:46:57 -0700 Subject: [PATCH 2/7] Fix the seven comment-based-help gaps the coverage sweep finds Documentation only -- no executable line changes, so nothing here can alter a request the module sends or a response it parses. Six cmdlets had undocumented parameters: Update-PfbSupport no help block at all; 0 of 2 parameters Set-PfbContext 0 of 5 Invoke-PfbInContext 0 of 5 Clear-PfbContext 0 of 1 Get-PfbLog 4 of 6 (StartTime, EndTime) Get-PfbRemoteArray 6 of 7 (CurrentFleetOnly) A seventh, not in the original list, is the opposite drift and is the reason the sweep matches by name rather than by count: Remove-PfbArrayErasure carried a .PARAMETER Attributes entry for a parameter its param() block does not declare. Removed rather than added -- the cmdlet issues a DELETE with no body, so the entry is stale rather than a missing feature. tools/Update-PfbContextHelp.ps1 was checked first for the three Context cmdlets, per the spec's note. It generates a .NOTES context-requirement block only, keyed on endpoint contextScope, and emits no .PARAMETER entries at all -- so these are hand-written and are not at risk of being overwritten by a regeneration. Public/ now reads 544/544 cmdlets with a .SYNOPSIS and 2860/2860 declared parameters documented, with no orphaned, duplicated, nameless or empty entries. Co-Authored-By: Claude Opus 5 (1M context) --- Public/Array/Remove-PfbArrayErasure.ps1 | 2 -- Public/Context/Clear-PfbContext.ps1 | 3 +++ Public/Context/Invoke-PfbInContext.ps1 | 15 +++++++++++++++ Public/Context/Set-PfbContext.ps1 | 15 +++++++++++++++ Public/Monitoring/Get-PfbLog.ps1 | 6 ++++++ Public/Replication/Get-PfbRemoteArray.ps1 | 5 +++++ Public/Support/Update-PfbSupport.ps1 | 19 +++++++++++++++++++ 7 files changed, 63 insertions(+), 2 deletions(-) diff --git a/Public/Array/Remove-PfbArrayErasure.ps1 b/Public/Array/Remove-PfbArrayErasure.ps1 index cd5b58b..6356e31 100644 --- a/Public/Array/Remove-PfbArrayErasure.ps1 +++ b/Public/Array/Remove-PfbArrayErasure.ps1 @@ -5,8 +5,6 @@ function Remove-PfbArrayErasure { .DESCRIPTION The Remove-PfbArrayErasure cmdlet deletes an array erasure job from the connected Pure Storage FlashBlade. This cmdlet has a high confirm impact. - .PARAMETER Attributes - A hashtable identifying the erasure job to remove. .PARAMETER Array The FlashBlade connection object. If not specified, the default connection is used. .EXAMPLE diff --git a/Public/Context/Clear-PfbContext.ps1 b/Public/Context/Clear-PfbContext.ps1 index 1caea77..173bc50 100644 --- a/Public/Context/Clear-PfbContext.ps1 +++ b/Public/Context/Clear-PfbContext.ps1 @@ -6,6 +6,9 @@ function Clear-PfbContext { Its own cmdlet rather than a -Clear switch, matching the Set-/Clear-PfbCredential precedent, and because @() must keep its distinct "run this one call locally" meaning at the Invoke-PfbInContext layer. Copy-on-write, like Set-PfbContext. No network call. + .PARAMETER Array + The FlashBlade connection to copy. Defaults to the current default connection. The + object passed in is never mutated -- the context-free copy is returned instead. #> [CmdletBinding()] [OutputType([PSCustomObject])] diff --git a/Public/Context/Invoke-PfbInContext.ps1 b/Public/Context/Invoke-PfbInContext.ps1 index 4d83266..0310273 100644 --- a/Public/Context/Invoke-PfbInContext.ps1 +++ b/Public/Context/Invoke-PfbInContext.ps1 @@ -14,6 +14,21 @@ function Invoke-PfbInContext { Non-pipeable by design: its pipeline payload would have to be the scriptblock, which no cmdlet emits. The blessed form is -Context (Get-PfbFleetMember -FleetName 'x').member.name + .PARAMETER Array + The FlashBlade connection the ambient context is pushed onto for the duration of the + block. Required -- pass a connection object from Connect-PfbArray. + .PARAMETER Context + One or more Fusion context names to apply for the duration of the block. Required. + Pass @() to run the block against the local array, which is distinct from setting no + context at all. + .PARAMETER ScriptBlock + The block to run under the context. Positional (position 0) and required. The previous + context is restored in a finally, so it survives an exception thrown inside the block. + .PARAMETER Kind + What the names in -Context identify: an individual array ('Array', the default), a + fleet ('Fleet'), or a topology group ('TopologyGroup'). + .PARAMETER AllArrays + Applies the fleet-wide "all arrays" context form instead of naming members individually. .NOTES Concurrency: .ContextOverride lives on the shared connection object, so concurrent workers pushing overrides on the SAME connection race. Set the context before forking diff --git a/Public/Context/Set-PfbContext.ps1 b/Public/Context/Set-PfbContext.ps1 index 92aebbc..0a17f19 100644 --- a/Public/Context/Set-PfbContext.ps1 +++ b/Public/Context/Set-PfbContext.ps1 @@ -22,6 +22,21 @@ function Set-PfbContext { failure is swallowed, leaving the locality indeterminate and this cmdlet permissive. Under a management-access policy that denies GET /admins it can cost ~3 round trips rather than 1. Worth knowing before calling this in a loop over many members: cheap, but not free. + .PARAMETER Array + The FlashBlade connection to copy. Defaults to the current default connection. The + object passed in is never mutated -- the modified copy is returned instead. + .PARAMETER Context + One or more Fusion context names to set on the returned connection. Accepts pipeline + input, including from Get-PfbFleetMember and Get-PfbFleet. Pass no value at all to get + the explicit "requires -Context" error; to run locally instead, use Clear-PfbContext. + .PARAMETER Kind + What the names in -Context identify: an individual array ('Array', the default), a + fleet ('Fleet'), or a topology group ('TopologyGroup'). + .PARAMETER AllArrays + Sets the fleet-wide "all arrays" context form instead of naming members individually. + .PARAMETER AllowErrors + Reserved for Phase 2. Accepted and stored on the returned connection; nothing is + injected on the wire for it yet. .NOTES Mixed-platform fleets: Get-PfbFleetMember will happily return FlashArrays. Piping those in is not supported -- cross-platform context is a non-goal (open question 5). diff --git a/Public/Monitoring/Get-PfbLog.ps1 b/Public/Monitoring/Get-PfbLog.ps1 index 5487b92..46788a5 100644 --- a/Public/Monitoring/Get-PfbLog.ps1 +++ b/Public/Monitoring/Get-PfbLog.ps1 @@ -6,6 +6,12 @@ function Get-PfbLog { The Get-PfbLog cmdlet returns log entries from the connected Pure Storage FlashBlade. Results can be narrowed using a server-side filter expression and sorted or limited as needed. + .PARAMETER StartTime + Start of the time window, in milliseconds since the Unix epoch. Defaults to one hour + ago. Always sent, whether or not it is supplied. + .PARAMETER EndTime + End of the time window, in milliseconds since the Unix epoch. Defaults to now. Always + sent, whether or not it is supplied. .PARAMETER Filter A server-side filter expression to narrow results (e.g., "severity='warning'"). .PARAMETER Sort diff --git a/Public/Replication/Get-PfbRemoteArray.ps1 b/Public/Replication/Get-PfbRemoteArray.ps1 index 539af39..cebec14 100644 --- a/Public/Replication/Get-PfbRemoteArray.ps1 +++ b/Public/Replication/Get-PfbRemoteArray.ps1 @@ -16,6 +16,11 @@ function Get-PfbRemoteArray { Sort field and direction (e.g., "name" or "name-"). .PARAMETER Limit Maximum number of entries to return. + .PARAMETER CurrentFleetOnly + Restricts results to remote arrays in the current fleet. Defaults to $true, so pass + -CurrentFleetOnly:$false to include arrays outside it. A scope flag rather than a + selector: it is written on every path and never counts as "a selector reached the + query". .PARAMETER Array The FlashBlade connection object. If not specified, the default connection is used. .EXAMPLE diff --git a/Public/Support/Update-PfbSupport.ps1 b/Public/Support/Update-PfbSupport.ps1 index ef95e35..4b1d22c 100644 --- a/Public/Support/Update-PfbSupport.ps1 +++ b/Public/Support/Update-PfbSupport.ps1 @@ -1,4 +1,23 @@ function Update-PfbSupport { + <# + .SYNOPSIS + Updates support configuration on a FlashBlade array. + .DESCRIPTION + The Update-PfbSupport cmdlet modifies the support configuration -- Phone Home, Remote + Assist and related settings -- on the connected FlashBlade. + .PARAMETER Attributes + A hashtable of support attributes to modify. + .PARAMETER Array + The FlashBlade connection object. If not specified, the default connection is used. + .EXAMPLE + Update-PfbSupport -Attributes @{ phonehome_enabled = $true } + + Enables Phone Home. + .EXAMPLE + Update-PfbSupport -Attributes @{ remote_assist_active = $false } -WhatIf + + Shows what would happen without actually updating the support configuration. + #> [CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'Medium')] param( [Parameter(Mandatory)] [hashtable]$Attributes, From 286ff4fdc27df63f26fa142396492575c1d0db09 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Tue, 25 Aug 2026 00:01:39 -0700 Subject: [PATCH 3/7] Address the branch review: close the fail-open paths in the two sweeps Eight code findings from the whole-branch review, plus the ConfirmImpact ruling the previous commit left open. Help-block lookup -- the one that could false-green: PfbHelpCoverage located a cmdlet's help by taking the nearest .SYNOPSIS block INSIDE the function, falling back to the nearest one ABOVE it. Two shapes then report as fully documented while Get-Help shows nothing: a file-header block a dozen lines up, and a block belonging to a nested helper when the cmdlet itself has none. The second is why dropping the fallback alone would not have been enough -- a nested function's extent is a subset of the cmdlet's, so those blocks passed the "inside" test too. A block must now sit inside the function body and outside any nested function within it. All 544 blocks in Public/ already satisfy that, so this is strictly stronger rather than a restriction anyone has to work around. The fallback branch had no fixture at all; four new fixtures now pin placement in both directions. ShouldContinue no longer satisfies SupportsShouldProcess: ShouldContinue does not participate in -WhatIf -- it is an extra confirmation prompt that runs regardless -- so a cmdlet declaring SupportsShouldProcess whose only guard is ShouldContinue still issues its request under -WhatIf, which is verbatim the harm that assertion exists to catch. The record now counts the two separately. ShouldContinue stays in the inner-scope scan, where the hazard is identical either way. sweep-tests-spec.md specified the original wording, so the implementation was faithful and the spec is what needed correcting. Remove-PfbWorkloadTag -- MAINTAINER RULING, keep it at Medium: The previous commit parked this as pending and said it must be settled before merge. Settled: a workload tag is metadata, DELETE /workloads/tags carries an empty body and destroys no array data, and the tag is cheaply recreated, so the prompt its 111 siblings carry would be friction protecting nothing. It is now a settled entry in $confirmImpactExempt carrying that reason. Accepted cost, recorded rather than glossed: someone who has learned that Remove-Pfb* stops and asks will not be asked here. Revisit if the endpoint ever gains the ability to delete something other than labels. The separate pending-decision list goes away with the ruling and its entry moves into $confirmImpactExempt, which keeps the same per-entry staleness guard: each name must resolve to exactly one real cmdlet, and that cmdlet must still be non-High. Neither list ever had a bound on its LENGTH -- a growth cap was requested in review and declined, so nothing mechanically stops N Remove-* cmdlets being parked at Medium with the sweep still green, and nothing enforces that an entry carries a written reason. That gap stays open on purpose; it is held as its own decision rather than folded into this ruling. Four fail-open paths closed: - A verb in none of the three lists was checked by nothing: not required to declare SupportsShouldProcess, not forbidden from declaring it. A census now reds once, names the offender, and turns classification into a one-line decision. Connect and Disconnect were sitting in that gap. - $shouldProcessExempt gained the staleness guard its neighbour already had. - The two copies of Test-PfbNestedInInnerScope were held in sync only by a comment, on a function both files call CI-critical and whose regression is caught by exactly one It in each. An It now asserts they are -ceq identical. - A comment claimed "first .SYNOPSIS wins" while the guard only prevented un-setting a flag that is never un-set, so a populated first .SYNOPSIS followed by an empty second flagged the cmdlet. Now actually enforced. coverage-baseline.psd1 gains both new Describes in both edition blocks. Assert-PfbTestCoverage.ps1 walks $Result.Containers, so a file that drops out of DISCOVERY entirely appears nowhere and the run stays green with the tripwire silently gone; RequiredDescribes is the only rail that sees that. The entries match the declared Describe names exactly, verified by comparison -- but they are first exercised end-to-end by CI, since this repo reserves the full-suite run for CI rather than running it locally. Also folds in a held docstring fix in scripts/Assert-PfbSpecCache.ps1, which cited .gitignore by line number (:36 and :46) and justified its own location with a blanket tools/ ignore that no longer exists. Comment text only. Verification: 17 tests pass under pwsh 7 and Windows PowerShell 5.1, 0 skipped, containers ok on both; 48/48 with the exemplar sweep and CiCoverageGate run alongside. Each of the eight new or changed assertions was mutation-tested and all eight were killed. The limit of that evidence is worth stating: neutering an offender FILTER survives by construction for any "the offender set is empty" assertion, since the set is empty either way on a clean tree. What protects those is the fixture pair and the population floors, not the mutation run. Co-Authored-By: Claude Opus 5 (1M context) --- Tests/PfbHelpCoverage.Tests.ps1 | 138 +++++++++++++++-- Tests/PfbShouldProcessCoverage.Tests.ps1 | 189 ++++++++++++++++++----- Tests/coverage-baseline.psd1 | 14 ++ scripts/Assert-PfbSpecCache.ps1 | 14 +- 4 files changed, 297 insertions(+), 58 deletions(-) diff --git a/Tests/PfbHelpCoverage.Tests.ps1 b/Tests/PfbHelpCoverage.Tests.ps1 index 1462a36..0d96e52 100644 --- a/Tests/PfbHelpCoverage.Tests.ps1 +++ b/Tests/PfbHelpCoverage.Tests.ps1 @@ -75,27 +75,51 @@ BeforeAll { # # A `.SYNOPSIS` keyword line is the marker (anchored, per Get-PfbHelpSection's reasoning), # so a .DESCRIPTION or .EXAMPLE that merely mentions the word cannot be mistaken for the - # block. Prefer a block INSIDE the function extent -- the convention throughout Public/ -- - # and fall back to the nearest block above the function, which is the other legal placement - # for comment-based help. Measured on main 2026-08-24: all 543 blocks are inside, so the - # fallback is future-proofing, not a live case. + # block. + # + # The block must sit INSIDE the function body, and must not belong to a nested helper. + # This is narrower than "a .SYNOPSIS somewhere near the function", deliberately: an earlier + # draft accepted the nearest block ABOVE the function as a fallback, and that admitted two + # shapes that report as fully documented while `Get-Help` shows nothing -- + # - a file-header block a dozen lines above the function, holding unrelated prose; + # - a block belonging to a nested helper, when the cmdlet itself has none. + # Both were probed against Get-Help; neither attaches. The `above` placement IS legal + # PowerShell, but only when adjacent, and a gate that cannot tell adjacent from distant is + # a gate that false-greens. All 544 blocks in Public/ are inside their function, so + # requiring that is strictly stronger here rather than a restriction anyone has to work + # around. The 'help-above-function' and 'nested-helper-only' fixtures below pin both. $candidates = @($Tokens | Where-Object { $_.Kind -eq [System.Management.Automation.Language.TokenKind]::Comment -and $_.Text -match '(?m)^\s*\.SYNOPSIS\s*$' }) - $inside = @($candidates | Where-Object { - $_.Extent.StartOffset -ge $Function.Extent.StartOffset -and - $_.Extent.EndOffset -le $Function.Extent.EndOffset - }) - $above = @($candidates | Where-Object { - $_.Extent.EndOffset -le $Function.Extent.StartOffset - }) + + $nestedFunctions = @($Function.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] + }, $true) | Where-Object { -not [object]::ReferenceEquals($_, $Function) }) $help = $null - if ($inside.Count -gt 0) { $help = $inside[0] } - elseif ($above.Count -gt 0) { $help = $above[-1] } + foreach ($candidate in $candidates) { + if ($candidate.Extent.StartOffset -lt $Function.Extent.StartOffset) { continue } + if ($candidate.Extent.EndOffset -gt $Function.Extent.EndOffset) { continue } + + $inNested = $false + foreach ($nested in $nestedFunctions) { + if ($candidate.Extent.StartOffset -ge $nested.Extent.StartOffset -and + $candidate.Extent.EndOffset -le $nested.Extent.EndOffset) { + $inNested = $true + break + } + } + if ($inNested) { continue } + + # Tokens arrive in source order, so the first surviving candidate is the cmdlet's own. + $help = $candidate + break + } $synopsisEmpty = $false + $seenSynopsis = $false $documented = @() $emptyParameterSections = @() $namelessParameterSections = 0 @@ -107,9 +131,15 @@ BeforeAll { $sectionBody = ($section.BodyLines -join "`n").Trim() if ($section.Keyword -eq 'SYNOPSIS') { - # First .SYNOPSIS wins; a second one is not a shape this tree produces. - if (-not $synopsisEmpty -and [string]::IsNullOrWhiteSpace($sectionBody)) { - $synopsisEmpty = $true + # First .SYNOPSIS wins -- enforced, not merely described. The previous guard + # was `-not $synopsisEmpty`, which only prevented un-setting a flag that is + # never un-set: given a populated first .SYNOPSIS and an empty second, the loop + # reached the second and flagged the cmdlet. Wrong direction is a false + # positive rather than a false green, but the comment claimed a behaviour the + # code did not have, which is the defect class 3c98a7f already paid for. + if (-not $seenSynopsis) { + $seenSynopsis = $true + $synopsisEmpty = [string]::IsNullOrWhiteSpace($sectionBody) } continue } @@ -425,6 +455,65 @@ function Get-PfbFixture { [CmdletBinding()] param([Parameter()] [string]$Name) } +'@ + # The three below pin the help-block LOOKUP itself, which nothing else here exercises: + # every fixture above puts the block inside the function, so the placement rule was the + # one predicate with no coverage in either direction. + 'help-above' = @' +<# +.SYNOPSIS + Adjacent help above the function. +.PARAMETER Name + The fixture name. +#> +function Get-PfbFixture { + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'distant-header' = @' +<# +.SYNOPSIS + File header for a script, not help for the function far below it. +.DESCRIPTION + Unrelated prose that documents the file rather than the cmdlet. +#> + +# Some other commentary sits between the header and the function. + +function Get-PfbFixture { + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'nested-only' = @' +function Get-PfbFixture { + [CmdletBinding()] + param([Parameter()] [string]$Name) + + function Get-PfbInnerHelper { + <# + .SYNOPSIS + Belongs to the nested helper, not to the cmdlet. + .PARAMETER Name + The helper's own parameter. + #> + param([string]$Name) + } +} +'@ + 'second-synopsis' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + A populated first synopsis. + .PARAMETER Name + The fixture name. + .SYNOPSIS + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} '@ } @@ -488,5 +577,22 @@ function Get-PfbFixture { $records['case-mismatch'].MissingParameters | Should -BeNullOrEmpty -Because 'PowerShell parameter names are case-insensitive, so .PARAMETER name documents $Name' $records['case-mismatch'].OrphanedParameters | Should -BeNullOrEmpty + + # Placement. All three of these read as fully documented to a lookup that accepts the + # nearest .SYNOPSIS above the function, and Get-Help attaches nothing to any of them. + $records['help-above'].HasHelpBlock | + Should -BeFalse -Because 'the convention throughout Public/ is a block inside the function body, and a lookup that also accepts an adjacent block above cannot distinguish it from a distant file header' + $records['help-above'].MissingParameters | Should -Be @('Name') + + $records['distant-header'].HasHelpBlock | + Should -BeFalse -Because 'a file-header block a dozen lines up documents the file, and Get-Help attaches none of it to the function' + + $records['nested-only'].HasHelpBlock | + Should -BeFalse -Because 'the only help block belongs to a nested helper, so the cmdlet itself is undocumented' + $records['nested-only'].MissingParameters | + Should -Be @('Name') -Because "the nested helper's .PARAMETER entry must not be credited to the cmdlet" + + $records['second-synopsis'].SynopsisEmpty | + Should -BeFalse -Because 'the first .SYNOPSIS is populated and wins; a later empty one must not flag the cmdlet' } } diff --git a/Tests/PfbShouldProcessCoverage.Tests.ps1 b/Tests/PfbShouldProcessCoverage.Tests.ps1 index 194c702..2d11858 100644 --- a/Tests/PfbShouldProcessCoverage.Tests.ps1 +++ b/Tests/PfbShouldProcessCoverage.Tests.ps1 @@ -17,6 +17,7 @@ # Nothing here imports the module. Parsing only, so no module state is created or leaked. BeforeAll { + $script:testRoot = $PSScriptRoot $script:moduleRoot = Split-Path -Parent $PSScriptRoot $script:publicRoot = Join-Path $script:moduleRoot 'Public' @@ -113,14 +114,25 @@ BeforeAll { # dynamic member name ($PSCmdlet.$verb(...)) is a MemberExpression rather than a string # constant -- guard the cast instead of stringifying, or a dynamic call would compare equal # to nothing and be silently uncounted. - $calls = @($Function.FindAll({ + $guardCalls = @($Function.FindAll({ param($node) if ($node -isnot [System.Management.Automation.Language.InvokeMemberExpressionAst]) { return $false } if ($node.Member -isnot [System.Management.Automation.Language.StringConstantExpressionAst]) { return $false } return ($node.Member.Value -in @('ShouldProcess', 'ShouldContinue')) }, $true)) - $nestedCalls = @($calls | Where-Object { + # COUNTED SEPARATELY, and the distinction is the whole point of assertion 2. + # ShouldContinue does NOT participate in -WhatIf: it is an extra confirmation prompt that + # runs regardless of it. So a cmdlet declaring SupportsShouldProcess whose only guard is + # ShouldContinue still issues its request under -WhatIf -- verbatim the harm assertion 2 + # exists to catch. Lumping the two together admitted exactly that defect. + # + # ShouldContinue stays in the NESTING scan below: a guard of either kind in an inner scope + # is the same control-flow hazard. + $shouldProcessCalls = @($guardCalls | Where-Object { $_.Member.Value -eq 'ShouldProcess' }) + $shouldContinueCalls = @($guardCalls | Where-Object { $_.Member.Value -eq 'ShouldContinue' }) + + $nestedCalls = @($guardCalls | Where-Object { Test-PfbNestedInInnerScope -Node $_ -Stop $Function }) @@ -131,7 +143,8 @@ BeforeAll { Line = $Function.Extent.StartLineNumber SupportsShouldProcess = $supportsShouldProcess ConfirmImpact = $confirmImpact - ShouldProcessCalls = $calls.Count + ShouldProcessCalls = $shouldProcessCalls.Count + ShouldContinueCalls = $shouldContinueCalls.Count NestedCalls = $nestedCalls.Count NestedCallLines = @($nestedCalls | ForEach-Object { $_.Extent.StartLineNumber }) } @@ -171,6 +184,14 @@ BeforeAll { # that does not exist -- -WhatIf output claiming a Get- cmdlet would change something. $script:readOnlyVerbs = @('Get', 'Test') + # Verbs that are deliberately neither. Each one is a decision, recorded here so the census + # assertion below can be exhaustive rather than a fail-open default: + # Invoke -- the three Invoke-* cmdlets are a scoping wrapper and two GET diagnostics + # (verified: -Method GET, and Invoke-PfbInContext issues no request at all) + # Connect -- session setup, no array mutation + # Disconnect -- session teardown, no array mutation + $script:otherVerbs = @('Invoke', 'Connect', 'Disconnect') + # A LITERAL list, never a name-shaped regex. A pattern like '*Context*' or '*Credential*' would # silently absorb a future cmdlet that genuinely does need a guard, which is precisely the decay # this file exists to catch. @@ -191,26 +212,22 @@ BeforeAll { 'Invoke-PfbNetworkTrace' # read-only diagnostic ) - # PENDING MAINTAINER DECISION -- NOT a settled exemption. + # Settled exemptions from the Remove-*-is-High rule. A LITERAL list, one written reason per + # entry, same discipline as $script:shouldProcessExempt above. # - # Remove-PfbWorkloadTag is the only Remove-* in the module declaring ConfirmImpact = 'Medium' - # rather than 'High'. At Medium it deletes without ever prompting, because the default - # $ConfirmPreference is High. That is either: - # (a) a real defect -- the cmdlet should be 'High' like its 111 siblings; or - # (b) deliberate -- a workload TAG is metadata, cheaply recreated, and unlike the other - # Remove-* cmdlets its loss destroys no data. + # Remove-PfbWorkloadTag is the only Remove-* of 112 declaring 'Medium' rather than 'High', so + # it is the only one that deletes without prompting ($ConfirmPreference defaults to High). + # Maintainer ruling 2026-08-24: KEEP it at Medium. A workload tag is metadata -- DELETE + # /workloads/tags removes label rows and destroys no array data, and the tag is cheaply + # recreated -- so the prompt its 111 siblings carry would be friction protecting nothing. + # The cost accepted with that ruling is a UX inconsistency: someone who has learned that + # Remove-Pfb* stops and asks will not be asked here. That errs toward fewer surprise prompts, + # which is the tolerable direction. # - # Which of those is true is the maintainer's call, not this test's, and it MUST be settled - # before this file merges. It is parked here, in its own list with its own name, rather than - # buried in $script:confirmImpactExempt, so that resolving it is a visible one-line edit: - # either move it into the settled list with a reason, or fix the cmdlet and delete this list. - $script:confirmImpactPendingDecision = @( - 'Remove-PfbWorkloadTag' + # Revisit if the endpoint ever grows the ability to delete something other than labels. + $script:confirmImpactExempt = @( + 'Remove-PfbWorkloadTag' # metadata only; DELETE /workloads/tags destroys no array data ) - - # Settled exemptions from the Remove-*-is-High rule. Empty today, and it should stay that way - # unless a specific cmdlet earns an entry with a written reason. - $script:confirmImpactExempt = @() } Describe 'ShouldProcess coverage' { @@ -239,6 +256,27 @@ Describe 'ShouldProcess coverage' { Should -BeGreaterOrEqual 100 -Because 'the ConfirmImpact assertion is scoped to Remove-*, so it needs its own floor' } + It 'classifies every verb in Public/, so a new one cannot arrive unnoticed' { + # The verb lists are allowlists, and an allowlist that is consulted but never checked for + # completeness FAILS OPEN: a cmdlet whose verb is in none of the three lists is invisible + # to every assertion below -- not required to declare SupportsShouldProcess, not forbidden + # from declaring it, not subject to the ConfirmImpact rule. `Restore-`, `Enable-`, + # `Import-` and `Deny-` would all land in that gap silently. + # + # Tests/PfbEmptyPipelineGuardCoverage.Tests.ps1 solves this by failing CLOSED (Get and Test + # are read verbs, everything else is state-changing). This file keeps the explicit lists, + # because the classification genuinely differs per verb here -- but then it owes a census, + # or it is the weaker of two conventions living side by side. + # + # This reds ONCE when a new verb appears, names it, and turns classification into a + # one-line decision. Measured on main 2026-08-24: Get 214, New 114, Remove 112, Update 82, + # Test 10, Set 4, Invoke 3, Clear 2, Add 1, Connect 1, Disconnect 1 = 544. + $known = @($script:stateChangingVerbs) + @($script:readOnlyVerbs) + @($script:otherVerbs) + $unclassified = @($script:cmdlets | Where-Object { $_.Verb -notin $known }) + $detail = @($unclassified | ForEach-Object { "$($_.File): $($_.Function) [verb = $($_.Verb)]" }) -join "`n" + $detail | Should -BeNullOrEmpty -Because "a verb in none of the three lists is checked by nothing in this file; classify it as state-changing, read-only or deliberately neither; offenders:`n$detail" + } + It 'declares SupportsShouldProcess on every state-changing cmdlet' { $offenders = @($script:cmdlets | Where-Object { $_.Verb -in $script:stateChangingVerbs -and @@ -247,17 +285,35 @@ Describe 'ShouldProcess coverage' { }) $detail = @($offenders | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" $detail | Should -BeNullOrEmpty -Because "a state-changing cmdlet without SupportsShouldProcess silently ignores -WhatIf and -Confirm; offenders:`n$detail" + + # Keep the exemption list honest, the same way the pending-decision list below is kept + # honest. A renamed or retired entry becomes a standing silent exemption for whatever + # future cmdlet takes that name -- and an entry whose cmdlet has since GAINED + # SupportsShouldProcess no longer needs exempting at all. + foreach ($name in $script:shouldProcessExempt) { + $record = @($script:cmdlets | Where-Object { $_.Function -eq $name }) + $record.Count | + Should -Be 1 -Because "the exemption '$name' must still name exactly one real cmdlet, or it is exempting nothing and shadowing a future name" + $record[0].SupportsShouldProcess | + Should -BeFalse -Because "'$name' now declares SupportsShouldProcess, so its exemption is stale and must be deleted" + } } It 'actually calls ShouldProcess wherever it declares SupportsShouldProcess' { # A declaration with no call is worse than no declaration: -WhatIf binds successfully, the # caller believes nothing happened, and the request went out anyway. Measured 0 violations # on main -- this is a tripwire protecting a clean state, not a fix for a live defect. + # + # ShouldProcessCalls counts ShouldProcess ONLY -- see Get-PfbShouldProcessRecord. + # ShouldContinue does not satisfy this: it ignores -WhatIf entirely, so a cmdlet guarded + # only by ShouldContinue produces precisely the failure described above. $offenders = @($script:cmdlets | Where-Object { $_.SupportsShouldProcess -and $_.ShouldProcessCalls -eq 0 }) - $detail = @($offenders | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" - $detail | Should -BeNullOrEmpty -Because "SupportsShouldProcess without a ShouldProcess call makes -WhatIf silently perform the operation; offenders:`n$detail" + $detail = @($offenders | ForEach-Object { + "$($_.File): $($_.Function) [ShouldContinue calls: $($_.ShouldContinueCalls)]" + }) -join "`n" + $detail | Should -BeNullOrEmpty -Because "SupportsShouldProcess without a ShouldProcess call makes -WhatIf silently perform the operation, and ShouldContinue does not count because it does not participate in -WhatIf; offenders:`n$detail" } It 'keeps every ShouldProcess call out of an inner scope' { @@ -305,28 +361,34 @@ Describe 'ShouldProcess coverage' { # invisible interactively for the cmdlets that DO prompt, so a sweep is the only place it # can be caught. # - # $script:confirmImpactPendingDecision is NOT a settled exemption -- see its definition. $offenders = @($script:cmdlets | Where-Object { $_.Verb -eq 'Remove' -and $_.ConfirmImpact -ne 'High' -and - $_.Function -notin $script:confirmImpactExempt -and - $_.Function -notin $script:confirmImpactPendingDecision + $_.Function -notin $script:confirmImpactExempt }) $detail = @($offenders | ForEach-Object { "$($_.File): $($_.Function) [ConfirmImpact = $($_.ConfirmImpact)]" }) -join "`n" $detail | Should -BeNullOrEmpty -Because "a Remove- cmdlet below ConfirmImpact 'High' deletes without ever prompting, because `$ConfirmPreference defaults to High; offenders:`n$detail" - # Keep the pending-decision list honest in both directions. If somebody resolves - # Remove-PfbWorkloadTag by raising it to High but forgets to empty this list, the entry - # becomes a silent standing exemption for a cmdlet that no longer needs one -- and the next - # Remove-* to regress to Medium under that same name would pass. So assert every parked - # name is still genuinely non-High. - foreach ($name in $script:confirmImpactPendingDecision) { + # Keep the exemption list honest in both directions, exactly as assertion 1 does for + # $script:shouldProcessExempt. If somebody later raises an exempted cmdlet to High but + # forgets to delete its entry, that entry becomes a silent standing exemption for a cmdlet + # that no longer needs one -- and the next Remove-* to regress to Medium under that same + # name would sail through the assertion above. + # + # There is deliberately no cap on this list's LENGTH. An earlier draft parked + # Remove-PfbWorkloadTag in a separate pending-decision list and capped that list at one + # entry, because a parking space that can grow becomes a general suppression mechanism for + # the load-bearing assertion. That hazard is gone with the parking space: an entry HERE is + # a written ruling with a stated reason, which is a decision rather than a deferral of one. + # If this list ever grows a reasonless entry, that is the thing to reject in review. + foreach ($name in $script:confirmImpactExempt) { $record = @($script:cmdlets | Where-Object { $_.Function -eq $name }) - $record.Count | Should -Be 1 -Because "the pending-decision entry '$name' must still name a real cmdlet" + $record.Count | + Should -Be 1 -Because "the ConfirmImpact exemption '$name' must still name exactly one real cmdlet, or it is exempting nothing and shadowing a future name" $record[0].ConfirmImpact | - Should -Not -Be 'High' -Because "'$name' is now High, so its pending-decision entry is stale and must be deleted" + Should -Not -Be 'High' -Because "'$name' is now High, so its exemption is stale and must be deleted" } } @@ -381,7 +443,8 @@ function Remove-PfbFixture { $node.Member -is [System.Management.Automation.Language.StringConstantExpressionAst] -and $node.Member.Value -in @('ShouldProcess', 'ShouldContinue') }, $true)) - $calls.Count | Should -Be 3 -Because 'ShouldContinue must be recognised alongside ShouldProcess' + $calls.Count | + Should -Be 3 -Because 'the NESTING scan covers both kinds -- a ShouldContinue in an inner scope is the same control-flow hazard as a ShouldProcess in one' $answers = @($calls | ForEach-Object { Test-PfbNestedInInnerScope -Node $_ -Stop $fixtureFunction @@ -394,11 +457,49 @@ function Remove-PfbFixture { $record.SupportsShouldProcess | Should -BeTrue -Because 'the attribute walk must read the expression-omitted form the whole tree uses' $record.ConfirmImpact | Should -Be 'High' - $record.ShouldProcessCalls | Should -Be 3 + # 2, not 3: the fixture's third call is a ShouldContinue, and ShouldProcessCalls counts + # only ShouldProcess. The split is what makes assertion 2 mean what its comment says. + $record.ShouldProcessCalls | + Should -Be 2 -Because 'the fixture has two ShouldProcess calls; the ShouldContinue is counted separately' + $record.ShouldContinueCalls | Should -Be 1 $record.NestedCalls | Should -Be 2 $record.Verb | Should -Be 'Remove' } + It 'keeps its copy of Test-PfbNestedInInnerScope byte-identical to the empty-pipeline sweep' { + # The duplication is deliberate and documented at the definition, but until now the only + # thing holding the two copies in sync was a comment asking the next editor to remember -- + # on a function both files call CI-critical, and whose regression is caught by exactly one + # It in each file. A comment is not a rail. Six lines and no import make it one. + # + # When the shared copy finally moves to tools/lib/, this It is what tells you the move is + # complete rather than half-done: it fails the moment the two texts diverge, including + # when one file starts calling a shared copy and the other still carries its own. + $thisFile = Join-Path $script:testRoot 'PfbShouldProcessCoverage.Tests.ps1' + $otherFile = Join-Path $script:testRoot 'PfbEmptyPipelineGuardCoverage.Tests.ps1' + $otherFile | Should -Exist + + $extract = { + param($Path) + $ast = [System.Management.Automation.Language.Parser]::ParseFile($Path, [ref]$null, [ref]$null) + $fn = @($ast.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and + $node.Name -eq 'Test-PfbNestedInInnerScope' + }, $true)) + $fn.Count | Should -Be 1 -Because "exactly one Test-PfbNestedInInnerScope must be defined in $(Split-Path -Leaf $Path)" + return $fn[0].Extent.Text + } + + $mine = & $extract $thisFile + $theirs = & $extract $otherFile + + # -ceq: a case-only difference is still a divergence between two copies that must stay + # identical, and -eq would not see it. + ($mine -ceq $theirs) | + Should -BeTrue -Because "the two copies of Test-PfbNestedInInnerScope have diverged; edit both or complete the extraction to tools/lib/`n--- this file ---`n$mine`n--- $(Split-Path -Leaf $otherFile) ---`n$theirs" + } + It 'recognises the negative shapes it is meant to flag' { # The mirror of the It above: prove each assertion's PREDICATE fires on a cmdlet that has # the defect, not just that no cmdlet in Public/ has it. Without this, an extraction bug @@ -439,6 +540,13 @@ function Get-PfbFixture { param([Parameter()] [string]$Name) if ($PSCmdlet.ShouldProcess('x')) { Invoke-PfbApiRequest -Method GET -Endpoint 'x' } } +'@ + 'shouldcontinue-only' = @' +function Remove-PfbFixture { + [CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'High')] + param([Parameter()] [string]$Name) + if ($PSCmdlet.ShouldContinue('x', 'caption')) { Invoke-PfbApiRequest -Method DELETE -Endpoint 'x' } +} '@ } @@ -461,6 +569,17 @@ function Get-PfbFixture { $records['declared-never-called'].SupportsShouldProcess | Should -BeTrue $records['declared-never-called'].ShouldProcessCalls | Should -Be 0 + # Assertion 2's predicate again, in the shape that used to slip through. This cmdlet has a + # guard, it prompts, and it reads as protective in review -- but ShouldContinue ignores + # -WhatIf, so `Remove-PfbFixture -WhatIf` still issues the DELETE. It must count as an + # offender, which means ShouldProcessCalls must be 0 while a guard call plainly exists. + $records['shouldcontinue-only'].SupportsShouldProcess | Should -BeTrue + $records['shouldcontinue-only'].ShouldProcessCalls | + Should -Be 0 -Because 'ShouldContinue does not participate in -WhatIf, so it cannot satisfy SupportsShouldProcess' + $records['shouldcontinue-only'].ShouldContinueCalls | Should -Be 1 + $records['shouldcontinue-only'].NestedCalls | + Should -Be 0 -Because 'the guard is a direct statement of the cmdlet block, so it is not an inner-scope finding' + # Assertion 5's predicate, in both the wrong-value and the omitted-value shapes. The # omitted shape matters on its own: a missing ConfirmImpact defaults to Medium at runtime, # so `$null -ne 'High'` has to count as an offender. diff --git a/Tests/coverage-baseline.psd1 b/Tests/coverage-baseline.psd1 index 2e7ff62..862fa8a 100644 --- a/Tests/coverage-baseline.psd1 +++ b/Tests/coverage-baseline.psd1 @@ -116,6 +116,14 @@ # to catch. 'Empty-pipeline guard coverage' 'Update-PfbEmptyPipelineGuards - real tree' + # The two Public/-population AST sweeps, same profile as the coverage block above: + # ungated, reading only committed *.ps1 files, contributing executed tests on every + # leg. The empty-file rail in scripts/Assert-PfbTestCoverage.ps1 cannot substitute -- + # it walks $Result.Containers, so a file that drops out of DISCOVERY entirely + # (renamed, moved, excluded by a path filter) appears nowhere and the run stays green + # with the tripwire silently gone. This list is the only rail that sees that. + 'ShouldProcess coverage' + 'Comment-based help coverage' # Issue #112 contextScope version tripwire. PS7-gated (it parses every cached spec # with ConvertFrom-Json -Depth), so it belongs to this block alone. It is exactly # what this list exists for: it is VACUOUSLY green today -- every endpoint declaring @@ -288,6 +296,12 @@ # red. Measured 7 passed / 15 passed on 5.1 for the two files. 'Empty-pipeline guard coverage' 'Update-PfbEmptyPipelineGuards - real tree' + # The two Public/-population AST sweeps -- see the pwsh7 block for the rationale. + # Both belong on this leg too: neither imports the module, neither reads the spec + # cache, and neither uses PS7-only syntax, so both run unskipped on 5.1 and + # requiring them here is not a false red. + 'ShouldProcess coverage' + 'Comment-based help coverage' # Issue #112, synthetic half only. It reads no spec and uses no PS7-only syntax, so # it runs on this leg (measured 7 passed on 5.1) and is the ONLY thing proving the # comparison can produce a finding at all -- the real-spec half it guards is diff --git a/scripts/Assert-PfbSpecCache.ps1 b/scripts/Assert-PfbSpecCache.ps1 index e14d770..c5a3ba0 100644 --- a/scripts/Assert-PfbSpecCache.ps1 +++ b/scripts/Assert-PfbSpecCache.ps1 @@ -2,8 +2,8 @@ .SYNOPSIS Fails the build when tools/specs/ did not materialise. .DESCRIPTION - Issue #63: tools/specs/ is a ~50MB cache of raw OpenAPI specs, gitignored (.gitignore:36 - and again .gitignore:46) because it is a build input rather than source. On a bare runner + Issue #63: tools/specs/ is a ~50MB cache of raw OpenAPI specs, gitignored (.gitignore, + "Cached raw OpenAPI specs") because it is a build input rather than source. On a bare runner it is therefore absent, and every tooling test that depends on it skipped gracefully while the job reported success -- roughly 23% of the suite, invisible in the run summary. @@ -12,11 +12,11 @@ red build rather than a quietly hollow test run. Without it the job would "succeed" and hand the test legs an empty directory, reproducing the exact defect being fixed. - Lives in scripts/ rather than tools/ deliberately: .gitignore:46 ignores tools/ wholesale - ("Not yet decided whether this should be tracked -- excluded for now"). The existing - tools/*.ps1 files are tracked only because they predate that rule, so a NEW file added - there would be silently untracked. scripts/ is unignored and already holds the scripts the - workflows call (see scripts/Publish-Gallery.ps1, used by publish-to-gallery.yml). + Lives in scripts/ rather than tools/ because scripts/ holds the scripts the workflows call + (see scripts/Publish-Gallery.ps1, used by publish-to-gallery.yml), while tools/ holds the + generators. Note that the blanket `tools/` ignore this file originally worked around is + gone: tools/specs/ is now the only tools/ exclusion, and the generator scripts and libs + under tools/ are tracked. .PARAMETER SpecsDirectory Defaults to tools/specs relative to the repo root. .PARAMETER MinimumCount From c47cae83d14b4743892f35cef2acf07882338c07 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Tue, 25 Aug 2026 08:36:13 -0700 Subject: [PATCH 4/7] Score help only where Get-Help actually reads it The previous commit claimed to close the help-lookup false-green, and it did not. Its rule was "the block is inside the function extent and outside any nested function", and a residual fail-open of exactly the class it set out to fix survived: a block one line BELOW param(), or stranded mid-body after an early return or a validation stanza, satisfies both conditions and Get-Help renders nothing for it. The sweep reported such a cmdlet as fully documented. It also took the FIRST surviving candidate on the stated grounds that tokens arrive in source order, which decides nothing about which block Get-Help reads. So this replaces the placement rule with a measured one. 34 probe shapes were written to disk, dot-sourced and read back through Get-Help -Full under both Windows PowerShell 5.1 and PowerShell 7.6.5 -- every shape agreed across the two editions, so there is one rule and not one per edition: - honoured positions are immediately ABOVE the function keyword (at most one blank line, nothing else in between), at the START of the body before the param block, and at the END of the body after the last statement. [CmdletBinding()] belongs to the param block, so a block between the attribute and param( is not honoured either. - ABOVE beats everything inside the body; START of body beats END of body; blocks on consecutive lines form one run and are concatenated, so a later .SYNOPSIS overrides an earlier one; a blank line ends the run and the first run carrying help keywords is the help. - a run whose keywords do not include .SYNOPSIS still claims the help and suppresses every later block -- .DESCRIPTION alone renders an EMPTY synopsis rather than falling through. That is why the keyword pattern matches any .KEYWORD: matching only .SYNOPSIS would credit a block Get-Help never reads. A block ABOVE the function is honoured by Get-Help -- the previous comment had that backwards, and the file contradicted itself about it -- but it stays a finding here, for two reasons now stated instead of asserted. It renders IN PLACE OF a block inside the body, so a cmdlet carrying both shows the outer one and the inner one is text no reader ever sees. And crediting it would mean this sweep has to agree with Get-Help about adjacency to the byte, where the failure direction is a file-header block being credited as a cmdlet's help. The record now carries HelpAboveFunction and HelpBlockMisplaced so the failure message can name the remedy -- move the block -- rather than telling a developer with perfectly good help to write some. One place where a first attempt at this was wrong in the other direction, kept here because it is the reason the rule is shaped as it is: a run that mixes a # line comment with the help block is not symmetric. A line comment ABOVE the block makes Get-Help render nothing; one BELOW it is honoured, with the line's text appended to the last section. Public/FileSystem/Get-PfbOpenFile.ps1 is the second shape -- it carries a drift-report note between the block and [CmdletBinding()] -- so rejecting both would have red-built a cmdlet whose help renders correctly, which the real file confirmed as a control. Also: the keyword regexes end in [ \t\r]*$ rather than [ \t]*$. In multiline mode $ matches before the \n but not before the \r, so a tail without \r matches nothing at all in a CRLF file, which every file here is. The symptom was the entire 544-cmdlet population reading as undocumented while the same pattern worked on an LF fixture. Nine fixtures added, each pinning a measured shape rather than a reading of the docs: help-below-param, help-mid-body, help-body-end, two-inside-start-and-end, two-blocks-one-run, two-runs-blank-separated, linecomment-then-help, help-then-linecomment, above-plus-inside and above-description-only. The nested function walk stays, demoted to diagnosis: the position test already rejects a nested helper's block, and what the walk now decides is whether a cmdlet is told "no help" or "help in the wrong place", which are different fixes. Two comment corrections in PfbShouldProcessCoverage and one message reword: - the ConfirmImpact exemption comment retired a rail that never existed. It said a one-entry cap on the pending-decision list had been removed because it had no remaining job. Verified against 564f7a0: that list carried only a per-entry staleness loop and no length assertion anywhere. A growth cap was requested in review and declined. The comment now says so, and says plainly that the gap is open -- N Remove-* cmdlets could be parked at Medium and this file would stay green, and nothing mechanically enforces the "written reason" the paragraph leans on. - assertion 1's -Because said only that a cmdlet needs comment-based help with a .SYNOPSIS. Under the new placement rule a cmdlet can fail it while holding a perfectly good .SYNOPSIS, so the message now names the placement requirement and each offender says which of the three cases it is. Verification: 48 passed / 0 failed / 0 skipped under both pwsh 7.6.5 and Windows PowerShell 5.1, across PfbHelpCoverage, PfbShouldProcessCoverage, PfbEmptyPipelineGuardCoverage and CiCoverageGate. Five mutations applied, each anchor confirmed to match exactly once before running so a no-op is not read as a survivor, and all five killed by the fixture assertion intended: neutering the position filter, taking the first rather than the last .SYNOPSIS in a run, searching the trailing region before the leading one, dropping the above-the-function flag, and ignoring line-comment order within a run. No version bump and no CHANGELOG entry -- the maintainer's separate decision. Co-Authored-By: Claude Opus 5 (1M context) --- Tests/PfbHelpCoverage.Tests.ps1 | 537 +++++++++++++++++++++-- Tests/PfbShouldProcessCoverage.Tests.ps1 | 19 +- 2 files changed, 508 insertions(+), 48 deletions(-) diff --git a/Tests/PfbHelpCoverage.Tests.ps1 b/Tests/PfbHelpCoverage.Tests.ps1 index 0d96e52..095283b 100644 --- a/Tests/PfbHelpCoverage.Tests.ps1 +++ b/Tests/PfbHelpCoverage.Tests.ps1 @@ -56,6 +56,184 @@ BeforeAll { return $sections } + # Which comment block `Get-Help` actually reads, for a function. + # + # MEASURED, not assumed. 34 probe shapes were written to disk, dot-sourced, and read back + # through `Get-Help -Full` under BOTH Windows PowerShell 5.1 (5.1.26100.8875) and PowerShell + # 7.6.5. Every shape gave the same answer on both editions, so there is one rule to encode + # rather than one per edition. + # + # HONOURED positions: + # - immediately ABOVE the `function` keyword, separated by at most one blank line and by + # nothing else. Two blank lines breaks it; so does an intervening `#` line comment. + # - at the START of the body, BEFORE the param block. `[CmdletBinding()]` is part of the + # param block, so a block between the attribute and `param(` is NOT honoured, and neither + # is one immediately BELOW `param(` -- that one is the shape this lookup used to credit. + # - at the END of the body, after the last statement. Trailing blank lines are fine. + # NOT honoured: anywhere in the middle of the body, and anywhere inside a `process { }` or + # other sub-block. `Get-Help` renders its auto-generated syntax-only help instead. + # + # PRECEDENCE, when more than one block competes: + # - ABOVE beats every block inside the body, including one at the start of the body. + # - START of body beats END of body. + # - blocks on CONSECUTIVE lines form one run and are concatenated, so a later `.SYNOPSIS` + # overrides an earlier one: the LAST `.SYNOPSIS` in a run wins. + # - a blank line ends a run, and the FIRST run carrying any help keyword is the help. A run + # whose keywords do NOT include `.SYNOPSIS` still wins and suppresses every later block: + # `.DESCRIPTION` alone renders an EMPTY synopsis rather than falling through to a + # `.SYNOPSIS` further down. That is why the keyword pattern below is any `.KEYWORD` and + # not just `.SYNOPSIS` -- matching only `.SYNOPSIS` would credit a block `Get-Help` never + # reads. + # + # A `#` line comment inside a run cuts both ways, asymmetrically, and both directions were + # measured: one BEFORE the block comment makes `Get-Help` render nothing, while one AFTER it + # is honoured with the line's text appended to whatever section came last. Public/ contains + # the second shape, so the two cannot be collapsed. + # + # Two known places where this is deliberately narrower than `Get-Help`, both erring toward a + # red build rather than a false green: + # - line-comment-style help (`# .SYNOPSIS` on consecutive lines) is not recognised at all, + # above or inside. It is legal PowerShell and nothing in Public/ uses it. + # - a run whose only help keyword is one `Get-Help` does not recognise (`.FOO`) is treated as + # claiming the help, so nothing later is credited. `Get-Help` may well fall through to a + # later block; the cost of being wrong here is a red build on a shape nobody writes. + function Get-PfbHelpToken { + param( + [System.Management.Automation.Language.FunctionDefinitionAst]$Function, + [System.Management.Automation.Language.Token[]]$Tokens, + [string]$KeywordPattern, + [string]$SynopsisPattern + ) + + $commentKind = [System.Management.Automation.Language.TokenKind]::Comment + $newLineKind = [System.Management.Automation.Language.TokenKind]::NewLine + + # ABOVE the function keyword. Reported, not credited -- see the record's + # HelpAboveFunction field for why that is a finding rather than a pass. + foreach ($token in $Tokens) { + if ($token.Kind -ne $commentKind) { continue } + if ($token.Text -notlike '<#*') { continue } + if ($token.Text -notmatch $KeywordPattern) { continue } + if ($token.Extent.EndOffset -gt $Function.Extent.StartOffset) { continue } + + $between = @($Tokens | Where-Object { + $_.Extent.StartOffset -ge $token.Extent.EndOffset -and + $_.Extent.EndOffset -le $Function.Extent.StartOffset + }) + if (@($between | Where-Object { $_.Kind -ne $newLineKind }).Count -gt 0) { continue } + # One newline is adjacency, two is a single blank line -- both honoured. Three is two + # blank lines, which is not. + if ($between.Count -gt 2) { continue } + + return [PSCustomObject]@{ Token = $null; AboveFunction = $true } + } + + # Inside the body. Work from the TOKENS rather than the statement list: the param block, + # its `[CmdletBinding()]` attribute and a named `begin`/`process`/`end` block are all + # "code" for this purpose, and the token stream treats them uniformly. Body.Extent + # includes the braces, so the strict comparisons drop them. + $bodyStart = $Function.Body.Extent.StartOffset + $bodyEnd = $Function.Body.Extent.EndOffset + $inBody = @($Tokens | Where-Object { + $_.Extent.StartOffset -gt $bodyStart -and $_.Extent.EndOffset -lt $bodyEnd + }) + $code = @($inBody | Where-Object { $_.Kind -ne $commentKind -and $_.Kind -ne $newLineKind }) + + # A body with no code at all: every comment in it is simultaneously at the start and the + # end, and the leading region is searched first. + $codeStart = $bodyEnd + $codeEnd = $bodyStart + if ($code.Count -gt 0) { + $codeStart = $code[0].Extent.StartOffset + $codeEnd = $code[$code.Count - 1].Extent.EndOffset + } + + $comments = @($inBody | Where-Object { $_.Kind -eq $commentKind }) + $leading = @($comments | Where-Object { $_.Extent.EndOffset -le $codeStart }) + $trailing = @($comments | Where-Object { $_.Extent.StartOffset -ge $codeEnd }) + + $found = Select-PfbHelpFromRegion -Comments $leading -Tokens $Tokens ` + -KeywordPattern $KeywordPattern -SynopsisPattern $SynopsisPattern + if (-not $found.Stopped) { + $found = Select-PfbHelpFromRegion -Comments $trailing -Tokens $Tokens ` + -KeywordPattern $KeywordPattern -SynopsisPattern $SynopsisPattern + } + + return [PSCustomObject]@{ Token = $found.Token; AboveFunction = $false } + } + + # Pick the block `Get-Help` would read out of ONE honoured region (start of body, or end of + # body), per the precedence measured above. + # + # `Stopped` says the region held a help block, whether or not that block turned out to carry a + # `.SYNOPSIS` this sweep can score. It is what keeps the end-of-body search from crediting a + # block `Get-Help` never reaches, because a start-of-body block already claimed the help. + function Select-PfbHelpFromRegion { + param( + [System.Management.Automation.Language.Token[]]$Comments, + [System.Management.Automation.Language.Token[]]$Tokens, + [string]$KeywordPattern, + [string]$SynopsisPattern + ) + + $newLineKind = [System.Management.Automation.Language.TokenKind]::NewLine + + # Group into runs: consecutive lines are one run, a blank line starts a new one. + $runs = [System.Collections.Generic.List[object]]::new() + $current = $null + $previous = $null + foreach ($comment in $Comments) { + $sameRun = $false + if ($null -ne $previous) { + $gap = @($Tokens | Where-Object { + $_.Extent.StartOffset -ge $previous.Extent.EndOffset -and + $_.Extent.EndOffset -le $comment.Extent.StartOffset + }) + $sameRun = (@($gap | Where-Object { $_.Kind -ne $newLineKind }).Count -eq 0) -and + (@($gap | Where-Object { $_.Kind -eq $newLineKind }).Count -le 1) + } + if (-not $sameRun) { + $current = [System.Collections.Generic.List[object]]::new() + $runs.Add($current) + } + $current.Add($comment) + $previous = $comment + } + + foreach ($run in $runs) { + # A run of ordinary commentary is not help and does not suppress what follows it -- + # measured: a line comment, a blank line, then the real block, and Get-Help reads the + # block. So the keyword test comes FIRST, before the composition test below. + $keyworded = @($run | Where-Object { $_.Text -like '<#*' -and $_.Text -match $KeywordPattern }) + if ($keyworded.Count -eq 0) { continue } + + # A run carrying help keywords claims the help, so from here on every path stops. + # + # Position of a `#` line comment inside the run decides it, and the two directions are + # not symmetric -- measured, on both editions. A line comment BEFORE the block makes + # Get-Help render nothing at all. One AFTER the block is honoured, with the line's text + # appended to whichever section came last. Public/Get-PfbOpenFile.ps1 is the second + # shape (a drift-report note between the block and [CmdletBinding()]), so treating the + # two alike would red-build a cmdlet whose help renders correctly. + $firstKeyworded = $keyworded[0] + $precedingLineComment = @($run | Where-Object { + $_.Text -notlike '<#*' -and + $_.Extent.StartOffset -lt $firstKeyworded.Extent.StartOffset + }) + if ($precedingLineComment.Count -gt 0) { + return [PSCustomObject]@{ Token = $null; Stopped = $true } + } + + $synopsis = @($run | Where-Object { $_.Text -like '<#*' -and $_.Text -match $SynopsisPattern }) + if ($synopsis.Count -eq 0) { + return [PSCustomObject]@{ Token = $null; Stopped = $true } + } + return [PSCustomObject]@{ Token = $synopsis[$synopsis.Count - 1]; Stopped = $true } + } + + return [PSCustomObject]@{ Token = $null; Stopped = $false } + } + # Reduce one cmdlet to the facts the assertions below need. function Get-PfbHelpRecord { param( @@ -77,46 +255,58 @@ BeforeAll { # so a .DESCRIPTION or .EXAMPLE that merely mentions the word cannot be mistaken for the # block. # - # The block must sit INSIDE the function body, and must not belong to a nested helper. - # This is narrower than "a .SYNOPSIS somewhere near the function", deliberately: an earlier - # draft accepted the nearest block ABOVE the function as a fallback, and that admitted two - # shapes that report as fully documented while `Get-Help` shows nothing -- - # - a file-header block a dozen lines above the function, holding unrelated prose; - # - a block belonging to a nested helper, when the cmdlet itself has none. - # Both were probed against Get-Help; neither attaches. The `above` placement IS legal - # PowerShell, but only when adjacent, and a gate that cannot tell adjacent from distant is - # a gate that false-greens. All 544 blocks in Public/ are inside their function, so - # requiring that is strictly stronger here rather than a restriction anyone has to work - # around. The 'help-above-function' and 'nested-helper-only' fixtures below pin both. - $candidates = @($Tokens | Where-Object { - $_.Kind -eq [System.Management.Automation.Language.TokenKind]::Comment -and - $_.Text -match '(?m)^\s*\.SYNOPSIS\s*$' - }) + # The block must sit at a position `Get-Help` HONOURS, and it must be inside the function + # body. Get-PfbHelpToken above carries the measured placement and precedence rules; two + # points about how this gate uses them: + # + # Being inside the function extent is not enough, which is what this lookup used to check. + # A block below `param()`, or in the middle of a body after an early-return guard, is + # inside the extent and outside any nested function, and `Get-Help` shows nothing for it -- + # so the old test reported "fully documented" for a cmdlet with no rendered help at all. + # The 'help-below-param' and 'help-mid-body' fixtures pin that. + # + # A block immediately ABOVE the `function` keyword is honoured by `Get-Help` -- that much + # the earlier comment here had backwards -- but it is still a finding, and the convention + # is a block inside the body (all 544 in Public/ are). Two reasons to keep flagging it. + # It renders IN PLACE OF a block inside the body, so a cmdlet with both shows the outer + # one and the inner one is dead text no reader will ever see ('above-plus-inside' pins + # that Get-Help prefers the outer). And crediting it would mean this sweep has to agree + # with Get-Help about adjacency to the byte, where the failure direction is a file-header + # block getting credited as a cmdlet's help ('distant-header'). Reported separately, via + # HelpAboveFunction, so the failure message can name the actual remedy. + # `\r` is in the trailing character class on purpose. In multiline mode `$` matches before + # the `\n` but NOT before the `\r`, so a `[ \t]*$` tail silently matches nothing at all in + # a CRLF file -- which every file in this repo is. The symptom is the whole population + # reading as undocumented while the same pattern works on an LF fixture. + $keywordPattern = '(?m)^[ \t]*\.[A-Za-z]+(?:[ \t]+\S+)?[ \t\r]*$' + $synopsisPattern = '(?m)^[ \t]*\.SYNOPSIS[ \t\r]*$' + + $lookup = Get-PfbHelpToken -Function $Function -Tokens $Tokens ` + -KeywordPattern $keywordPattern -SynopsisPattern $synopsisPattern + $help = $lookup.Token + # Nested helpers, for DIAGNOSIS only. The position test above already rejects a nested + # helper's block (it can be neither before the outer body's first code token nor after its + # last), so this no longer gates the lookup -- it distinguishes "this cmdlet has no help" + # from "this cmdlet's help is in the wrong place", which are different fixes. $nestedFunctions = @($Function.FindAll({ param($node) $node -is [System.Management.Automation.Language.FunctionDefinitionAst] }, $true) | Where-Object { -not [object]::ReferenceEquals($_, $Function) }) - $help = $null - foreach ($candidate in $candidates) { - if ($candidate.Extent.StartOffset -lt $Function.Extent.StartOffset) { continue } - if ($candidate.Extent.EndOffset -gt $Function.Extent.EndOffset) { continue } - - $inNested = $false - foreach ($nested in $nestedFunctions) { - if ($candidate.Extent.StartOffset -ge $nested.Extent.StartOffset -and - $candidate.Extent.EndOffset -le $nested.Extent.EndOffset) { - $inNested = $true - break - } - } - if ($inNested) { continue } - - # Tokens arrive in source order, so the first surviving candidate is the cmdlet's own. - $help = $candidate - break - } + $ownCandidates = @($Tokens | Where-Object { + $_.Kind -eq [System.Management.Automation.Language.TokenKind]::Comment -and + $_.Text -match $synopsisPattern -and + $_.Extent.StartOffset -ge $Function.Extent.StartOffset -and + $_.Extent.EndOffset -le $Function.Extent.EndOffset + } | Where-Object { + $candidate = $_ + -not @($nestedFunctions | Where-Object { + $candidate.Extent.StartOffset -ge $_.Extent.StartOffset -and + $candidate.Extent.EndOffset -le $_.Extent.EndOffset + }).Count + }) + $misplaced = ($null -eq $help) -and (-not $lookup.AboveFunction) -and ($ownCandidates.Count -gt 0) $synopsisEmpty = $false $seenSynopsis = $false @@ -179,6 +369,8 @@ BeforeAll { Function = $Function.Name Line = $Function.Extent.StartLineNumber HasHelpBlock = ($null -ne $help) + HelpAboveFunction = $lookup.AboveFunction + HelpBlockMisplaced = $misplaced SynopsisEmpty = $synopsisEmpty DeclaredParameters = $declared DocumentedParameters = $documented @@ -252,9 +444,25 @@ Describe 'Comment-based help coverage' { # Presence and content in one assertion, because they fail together in practice: a block # that lost its text is the same defect as a block that was never written, and splitting # them would let a bare `.SYNOPSIS` line satisfy a presence check while documenting nothing. + # + # PLACEMENT is part of this assertion, so the message has to name it: a cmdlet can hold a + # perfectly good .SYNOPSIS and still fail here because it sits where `Get-Help` will not + # read it. Each offender is annotated with which of the three it is, because the remedy + # differs -- write the help, or move it. $missingBlock = @($script:cmdlets | Where-Object { -not $_.HasHelpBlock }) - $missingDetail = @($missingBlock | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" - $missingDetail | Should -BeNullOrEmpty -Because "every public cmdlet needs comment-based help with a .SYNOPSIS; offenders:`n$missingDetail" + $missingDetail = @($missingBlock | ForEach-Object { + $reason = if ($_.HelpAboveFunction) { + 'help block sits ABOVE the function keyword -- move it inside the body' + } + elseif ($_.HelpBlockMisplaced) { + 'help block is at a position Get-Help does not honour (mid-body, or below param()) -- move it to the top of the body' + } + else { + 'no help block at all' + } + "$($_.File): $($_.Function) -- $reason" + }) -join "`n" + $missingDetail | Should -BeNullOrEmpty -Because "every public cmdlet needs comment-based help that Get-Help will actually render: one block comment as the FIRST thing in the function body, above [CmdletBinding()] and param(). A .SYNOPSIS below param(), in the middle of the body, or above the function keyword does not satisfy this even though it is real help text -- move the existing block rather than writing a second one; offenders:`n$missingDetail" $emptySynopsis = @($script:cmdlets | Where-Object { $_.SynopsisEmpty }) $emptyDetail = @($emptySynopsis | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" @@ -456,9 +664,14 @@ function Get-PfbFixture { param([Parameter()] [string]$Name) } '@ - # The three below pin the help-block LOOKUP itself, which nothing else here exercises: - # every fixture above puts the block inside the function, so the placement rule was the - # one predicate with no coverage in either direction. + # The rest pin the help-block LOOKUP itself, which nothing else here exercises: every + # fixture above puts the block at the top of the function body, so placement and + # precedence had no coverage in either direction. ('second-synopsis' is the exception + # -- it pins the first-.SYNOPSIS-wins rule inside a single block, not the lookup.) + # + # Each of these was measured against real `Get-Help` output on both editions before + # being written down; the expectations below are the measurement, not a reading of the + # docs. Where this sweep deliberately differs from `Get-Help` the -Because says so. 'help-above' = @' <# .SYNOPSIS @@ -514,6 +727,170 @@ function Get-PfbFixture { [CmdletBinding()] param([Parameter()] [string]$Name) } +'@ + 'help-below-param' = @' +function Get-PfbFixture { + [CmdletBinding()] + param([Parameter()] [string]$Name) + <# + .SYNOPSIS + Real help text, one line too low to be read. + .PARAMETER Name + The fixture name. + #> + + Write-Output 'body' +} +'@ + 'help-mid-body' = @' +function Get-PfbFixture { + [CmdletBinding()] + param([Parameter()] [string]$Name) + + if (-not $Name) { throw 'Name is required' } + + <# + .SYNOPSIS + Real help text, stranded in the middle of the body. + .PARAMETER Name + The fixture name. + #> + + Write-Output 'body' +} +'@ + 'help-body-end' = @' +function Get-PfbFixture { + [CmdletBinding()] + param([Parameter()] [string]$Name) + + Write-Output 'body' + + <# + .SYNOPSIS + Help at the end of the body, which Get-Help does read. + .PARAMETER Name + The fixture name. + #> +} +'@ + 'two-inside-start-and-end' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + The block at the start of the body. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) + + Write-Output 'body' + + <# + .SYNOPSIS + The block at the end of the body, which Get-Help ignores in favour of the first. + .PARAMETER Other + A parameter this cmdlet does not declare. + #> +} +'@ + 'two-blocks-one-run' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + First block of the run. + .PARAMETER Other + A parameter this cmdlet does not declare. + #> + <# + .SYNOPSIS + Second block of the run, whose .SYNOPSIS overrides the first. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'two-runs-blank-separated' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + First run, which Get-Help reads. + .PARAMETER Name + The fixture name. + #> + + <# + .SYNOPSIS + Second run, which Get-Help never reaches. + .PARAMETER Other + A parameter this cmdlet does not declare. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'linecomment-then-help' = @' +function Get-PfbFixture { + # An ordinary line comment on the line directly above the help block. + <# + .SYNOPSIS + Help in a run that also holds a line comment. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'help-then-linecomment' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Help with a line comment directly BELOW it, as Public/Get-PfbOpenFile.ps1 has. + .PARAMETER Name + The fixture name. + #> + # A note to the reader, between the help block and [CmdletBinding()]. + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'above-plus-inside' = @' +<# +.SYNOPSIS + Adjacent help above the function, which Get-Help renders. +.PARAMETER Other + A parameter this cmdlet does not declare. +#> +function Get-PfbFixture { + <# + .SYNOPSIS + Help inside the body, which Get-Help never renders because of the block above. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'above-description-only' = @' +<# +.DESCRIPTION + An adjacent block above the function carrying a help keyword but no .SYNOPSIS. +#> +function Get-PfbFixture { + <# + .SYNOPSIS + Help inside the body, suppressed by the block above. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} '@ } @@ -578,14 +955,90 @@ function Get-PfbFixture { Should -BeNullOrEmpty -Because 'PowerShell parameter names are case-insensitive, so .PARAMETER name documents $Name' $records['case-mismatch'].OrphanedParameters | Should -BeNullOrEmpty - # Placement. All three of these read as fully documented to a lookup that accepts the - # nearest .SYNOPSIS above the function, and Get-Help attaches nothing to any of them. + # Placement, part 1: blocks OUTSIDE the function body. The two cases are not the same and + # the earlier version of this comment ran them together. Measured on both editions: + # 'help-above' DOES render through Get-Help -- adjacent-above is a legal, honoured + # placement -- while 'distant-header' renders nothing. This sweep declines to credit + # either, deliberately and for the reasons in Get-PfbHelpRecord, so 'help-above' is the + # one place where a red here is a convention finding rather than "Get-Help shows nothing". $records['help-above'].HasHelpBlock | - Should -BeFalse -Because 'the convention throughout Public/ is a block inside the function body, and a lookup that also accepts an adjacent block above cannot distinguish it from a distant file header' + Should -BeFalse -Because 'the convention throughout Public/ is a block inside the function body; an adjacent block above IS honoured by Get-Help, but it renders in place of any block inside the body, and crediting it would mean matching Get-Help on adjacency byte for byte with a distant file header as the failure mode' + $records['help-above'].HelpAboveFunction | + Should -BeTrue -Because 'the failure message must be able to say "move it inside" rather than "write some help"' $records['help-above'].MissingParameters | Should -Be @('Name') $records['distant-header'].HasHelpBlock | Should -BeFalse -Because 'a file-header block a dozen lines up documents the file, and Get-Help attaches none of it to the function' + $records['distant-header'].HelpAboveFunction | + Should -BeFalse -Because 'two blank lines or an intervening line comment breaks adjacency, so this is not the above-the-function shape at all' + + # An above-the-function block WINS over a block inside the body, so the inner one is text + # no reader ever sees. Both fixtures pin that, and the second pins why the keyword pattern + # is any .KEYWORD rather than .SYNOPSIS alone: measured, an adjacent block carrying only + # .DESCRIPTION makes Get-Help render an EMPTY synopsis instead of falling through to the + # .SYNOPSIS inside the body. + $records['above-plus-inside'].HasHelpBlock | + Should -BeFalse -Because 'Get-Help renders the block above the function, so the block inside the body is dead text' + $records['above-plus-inside'].HelpAboveFunction | Should -BeTrue + $records['above-plus-inside'].MissingParameters | + Should -Be @('Name') -Because 'the inner block must not be credited for a parameter whose help never renders' + + $records['above-description-only'].HasHelpBlock | + Should -BeFalse -Because 'a block with any help keyword claims the help even without a .SYNOPSIS, so the inner block is still suppressed' + $records['above-description-only'].HelpAboveFunction | Should -BeTrue + + # Placement, part 2: positions INSIDE the body. These are the false greens the earlier + # lookup produced -- both blocks are inside the function extent and outside any nested + # function, which was the whole test, and Get-Help renders nothing for either. + $records['help-below-param'].HasHelpBlock | + Should -BeFalse -Because 'measured on both editions: a block below param() is not honoured, and Get-Help falls back to auto-generated syntax help' + $records['help-below-param'].HelpBlockMisplaced | + Should -BeTrue -Because 'the help text exists and needs moving, which is a different fix from writing help that does not exist' + $records['help-below-param'].MissingParameters | Should -Be @('Name') + + $records['help-mid-body'].HasHelpBlock | + Should -BeFalse -Because 'a block stranded mid-body after a guard is not honoured, so a lookup that only checks "inside the function" reports fully documented while Get-Help shows nothing' + $records['help-mid-body'].HelpBlockMisplaced | Should -BeTrue + $records['help-mid-body'].MissingParameters | Should -Be @('Name') + + $records['nested-only'].HelpBlockMisplaced | + Should -BeFalse -Because 'a nested helper''s block is not the cmdlet''s own help placed badly, so the message must not tell the developer to move it' + + # End of body IS honoured -- measured -- so this must NOT be flagged. Without it the + # position test could be "the block is the first thing in the body" and still pass + # everything else here, which would red-build a legal placement. + $records['help-body-end'].HasHelpBlock | + Should -BeTrue -Because 'a block after the last statement is an honoured placement and Get-Help renders it' + $records['help-body-end'].MissingParameters | Should -BeNullOrEmpty + + # Precedence among honoured blocks. Each fixture documents the DECLARED parameter from the + # block Get-Help picks and a parameter that does not exist from the block it ignores, so + # picking the wrong one shows up as a Missing/Orphaned pair rather than as a pass. + $records['two-inside-start-and-end'].HasHelpBlock | Should -BeTrue + $records['two-inside-start-and-end'].MissingParameters | + Should -BeNullOrEmpty -Because 'start of body beats end of body, so the start block is the one scored' + $records['two-inside-start-and-end'].OrphanedParameters | + Should -BeNullOrEmpty -Because 'scoring the end block instead would orphan its .PARAMETER Other' + + $records['two-blocks-one-run'].HasHelpBlock | Should -BeTrue + $records['two-blocks-one-run'].MissingParameters | + Should -BeNullOrEmpty -Because 'blocks on consecutive lines are one run and a later .SYNOPSIS overrides an earlier one, so the SECOND is what Get-Help renders' + $records['two-blocks-one-run'].OrphanedParameters | Should -BeNullOrEmpty + + $records['two-runs-blank-separated'].HasHelpBlock | Should -BeTrue + $records['two-runs-blank-separated'].MissingParameters | + Should -BeNullOrEmpty -Because 'a blank line ends the run, and the FIRST run carrying help keywords is the one Get-Help reads' + $records['two-runs-blank-separated'].OrphanedParameters | Should -BeNullOrEmpty + + # A `#` line comment in the same run as the help block, in both orders. These are NOT + # symmetric and the pair is what stops the rule being written as "reject any mixed run": + # Public/Get-PfbOpenFile.ps1 is the second shape, so rejecting both would red-build a + # cmdlet whose help renders perfectly well. + $records['linecomment-then-help'].HasHelpBlock | + Should -BeFalse -Because 'measured: a line comment ABOVE the block makes Get-Help render nothing for the function' + $records['help-then-linecomment'].HasHelpBlock | + Should -BeTrue -Because 'measured: a line comment BELOW the block is honoured, its text appended to the last section' + $records['help-then-linecomment'].MissingParameters | Should -BeNullOrEmpty $records['nested-only'].HasHelpBlock | Should -BeFalse -Because 'the only help block belongs to a nested helper, so the cmdlet itself is undocumented' diff --git a/Tests/PfbShouldProcessCoverage.Tests.ps1 b/Tests/PfbShouldProcessCoverage.Tests.ps1 index 2d11858..3405590 100644 --- a/Tests/PfbShouldProcessCoverage.Tests.ps1 +++ b/Tests/PfbShouldProcessCoverage.Tests.ps1 @@ -377,12 +377,19 @@ Describe 'ShouldProcess coverage' { # that no longer needs one -- and the next Remove-* to regress to Medium under that same # name would sail through the assertion above. # - # There is deliberately no cap on this list's LENGTH. An earlier draft parked - # Remove-PfbWorkloadTag in a separate pending-decision list and capped that list at one - # entry, because a parking space that can grow becomes a general suppression mechanism for - # the load-bearing assertion. That hazard is gone with the parking space: an entry HERE is - # a written ruling with a stated reason, which is a decision rather than a deferral of one. - # If this list ever grows a reasonless entry, that is the thing to reject in review. + # There is no cap on this list's LENGTH, and the history there is worth stating accurately + # because an earlier version of this comment described a rail that never existed. A growth + # cap on the parking list was REQUESTED in review and DECLINED; it was never implemented. + # What 564f7a0 actually shipped was $script:confirmImpactPendingDecision holding one name, + # guarded per entry (it must name exactly one real cmdlet, and that cmdlet must still be + # non-High) and not at all by length. That list was then deleted along with the ruling on + # Remove-PfbWorkloadTag, whose entry moved into $script:confirmImpactExempt below -- which + # carries the same per-entry staleness guard and, likewise, no length bound. + # + # So the gap is real, not closed: N Remove-* cmdlets could be parked here at 'Medium' and + # this file would stay green, and nothing mechanically enforces the "written reason" the + # paragraph above leans on -- a reasonless entry is caught only by a human in review. That + # is a deliberate choice about where to put the check, not a property of the code. foreach ($name in $script:confirmImpactExempt) { $record = @($script:cmdlets | Where-Object { $_.Function -eq $name }) $record.Count | From 8a72ad1f422382ba6e8710eb9b2a6b1e9daa43d9 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Tue, 25 Aug 2026 13:30:22 -0700 Subject: [PATCH 5/7] Model comment RUN composition, not just block position be32412 measured where Get-Help reads help from and encoded it, and the review of it found three fail-open shapes still standing. All three are the same root cause: it modelled where a block SITS and never asked what else shared its comment run. Adjacent comments concatenate into one help block and a blank line ends the run, so a block at a perfectly honoured position renders nothing when the line above it is ordinary commentary -- and the sweep scored it as documented. The rule was re-derived from scratch rather than patched, because three separate written accounts of Get-Help placement during this work were each wrong and each corrected only by measurement, including statements in be32412's own message and comments. 125 probe shapes were written to disk, dot-sourced and read back through Get-Help -Full under both Windows PowerShell 5.1 (5.1.26100.8875) and PowerShell 7.6.5. Every shape gave the same placement answer on both editions, so there is one rule and not one per edition. - The unit is the RUN: comment tokens on consecutive lines, `#` line comments and `<#...#>` blocks alike, concatenated with each delimiter stripped IN PLACE and each line comment's leading `#` removed. Content sharing a delimiter's line survives, measured, so the delimiter LINE must not be dropped. - A run is help when its first non-blank line is a recognised directive line AND every directive-SHAPED line in it is recognised. Prose above the first directive voids it; below one it is that section's body. One unrecognised directive anywhere voids the whole block, .SYNOPSIS included. - Regions are searched above the function (last run only, at most one blank line away), then the start of the body, then the end. A region holding no help run suppresses nothing and the search moves on -- so a #region marker or a file header above a function costs nothing. A run that IS help claims it absolutely, even with no .SYNOPSIS in it. The three findings then fall out of one predicate rather than three patches: - above-function help followed by a `#` line comment. The comment JOINS the run and the outer block still renders, in place of any block inside the body. The old code rejected any non-newline token between block and keyword, so it ignored the outer block and credited the inner one -- rendered help and scored help came from different blocks. - an ordinary `<#...#>` block on the line above a help block. Non-directive text first voids the run, help block and all. - the same with an unknown-keyword block. Its blank-line-separated twin is now CREDITED, reversing the documented conservative case: an unrecognised keyword claims nothing, so a help block below it renders normally, and treating it as claiming red-built a shape that works. Measurement contradicted the file in three more places, and the measurement wins in each: - the LAST .SYNOPSIS in a run wins, not the first, within one block as much as across blocks. A trailing EMPTY .SYNOPSIS overrides a populated one and Get-Help renders a blank synopsis. Taking the first was a false green over exactly the defect the empty-synopsis assertion exists to catch. The second-synopsis fixture asserted the opposite and now asserts this. - a run is ONE help block, so every .PARAMETER in it renders no matter which comment carried it. Scoring the single token that held the winning .SYNOPSIS hid the other blocks' entries from the orphan and duplicate assertions; two-blocks-one-run's .PARAMETER Other is a genuine orphan and is now asserted as one. - a bare `.PARAMETER` does not merely fail to document its parameter, it voids the entire block. The NamelessParameterSections field and its assertion are therefore gone: once the run test is correct that counter can never be non-zero, and an assertion over a structurally-zero field cannot fail, which is the failure mode the anti-vacuous floor at the top of the file exists to prevent. The same source defect now fails the first assertion instead, earlier and harder, and HelpRunDefect names the offending line -- the one case a developer cannot diagnose by reading a block that looks correct. The LOW from the review is fixed in the same message. It told developers that help above the function is not rendered. It IS rendered; the finding is a convention one, because an outer block renders in place of an inner one and a cmdlet carrying both has inner help no reader ever sees. The message now says that, and says why crediting it would force this sweep to match Get-Help on adjacency to the byte. Sixteen fixtures added or changed, each measured on both editions first: above-linecomment-then-inside, ordinary-then-help-one-run, ordinary-blank-then-help, unknown-keyword-then-help-one-run, unknown-keyword-blank-then-help, prose-before-keyword, synopsis-with-inline-argument, linecomment-help-claims-run, linecomment-continues-run, invalid-above-falls-through, ordinary-start-help-at-end, delimiter-line-content, unknown-keyword-later-in-block, dotword-prose-voids-block, above-blank-line-still-adjacent and distant-block-falls-through, plus reversed expectations on second-synopsis, two-blocks-one-run and nameless. The last two new ones exist because a mutation of the above-the-function adjacency bound SURVIVED: distant-header looks like it covers that bound and does not, since its last run before the keyword is a line comment and the block above never gets an adjacency test at all. dotword-prose-voids-block is worth naming on its own. `.NET Core is mentioned here` in an .EXAMPLE body is prose to a reader and a malformed directive to Get-Help, because the directive pattern is `\w` and not `[A-Za-z]` -- so is `.5 is a fraction`. Its harmless twin is `.\tools\Update-PfbContextHelp.ps1 -WhatIf` in the dotted-prose fixture, where a backslash is not a word character and the block renders. The pair is the point. Verification: 48 passed / 0 failed / 0 skipped on both editions across PfbHelpCoverage, PfbShouldProcessCoverage, PfbEmptyPipelineGuardCoverage and CiCoverageGate, unchanged from be32412. The 544-cmdlet population is unchanged too -- 544 cmdlets, 542 with parameters, 2860 declared, 2860 documented, and zero findings of any kind under the stricter rule. Seventeen mutations of the detector were applied on both editions, each anchor confirmed to match exactly once first so a no-op is not read as a survivor, and every one was killed. Two of the seventeen survived their first run and are the reason four of the fixtures above exist. Separately, all 41 fixtures were put through real Get-Help on both editions and compared against the detector: 37 agree exactly and 4 diverge, all four being the deliberate above-the-function convention. No version bump and no CHANGELOG entry -- the maintainer's separate decision. Co-Authored-By: Claude Opus 5 (1M context) --- Tests/PfbHelpCoverage.Tests.ps1 | 955 +++++++++++++++++++++++++------- 1 file changed, 755 insertions(+), 200 deletions(-) diff --git a/Tests/PfbHelpCoverage.Tests.ps1 b/Tests/PfbHelpCoverage.Tests.ps1 index 095283b..79cc645 100644 --- a/Tests/PfbHelpCoverage.Tests.ps1 +++ b/Tests/PfbHelpCoverage.Tests.ps1 @@ -16,11 +16,39 @@ # a file that is otherwise pure parsing; it also resolves help from external MAML and from the # .NOTES blocks tools/Update-PfbContextHelp.ps1 injects, which is a different question from # "is the source comment correct". Token-stream parsing answers the source question directly. +# +# That applies to the TEST, not to the rule the test encodes. Where Get-Help reads help from was +# measured by calling real Get-Help on real files -- see the placement note on Get-PfbRenderedHelp +# -- because every prose account of it written during this work, this file's own comments included, +# turned out to be wrong in some detail. BeforeAll { $script:moduleRoot = Split-Path -Parent $PSScriptRoot $script:publicRoot = Join-Path $script:moduleRoot 'Public' + # The comment-based-help directives Get-Help RECOGNISES, split by arity. + # + # Measured two independent ways, agreeing exactly. Behaviourally: each candidate was written as + # the opening directive of a comment run, followed by a blank line and a real help block, and + # read back through `Get-Help -Full` on both editions -- a directive Get-Help recognises claims + # the help and the later block never renders, one it does not is ordinary commentary and the + # later block renders normally. Then structurally: both editions' comment parser carries exactly + # these fifteen names, and the same directive pattern used in Test-PfbHelpRun below, so the list + # is complete for the two editions this repo runs on rather than merely unfalsified. + # + # ARITY is load-bearing, not decoration, and it cuts both ways. `.SYNOPSIS Some text` is NOT a + # directive line -- the argument-less ones must stand alone -- so a block opening with it is + # prose as far as Get-Help is concerned and renders nothing. A bare `.PARAMETER` is not a + # directive line either, for the mirror reason. + $script:helpDirectiveBare = @( + 'SYNOPSIS', 'DESCRIPTION', 'NOTES', 'LINK', 'ROLE', + 'EXAMPLE', 'OUTPUTS', 'INPUTS', 'COMPONENT', 'FUNCTIONALITY' + ) + $script:helpDirectiveArgument = @( + 'PARAMETER', 'FORWARDHELPTARGETNAME', 'FORWARDHELPCATEGORY', + 'REMOTEHELPRUNSPACE', 'EXTERNALHELP' + ) + # Split a comment-based help block into its sections. # # Returns one record per help keyword, in source order, carrying the keyword, its argument @@ -35,13 +63,13 @@ BeforeAll { [string]$Text ) - # Strip the block delimiters so `<#` and `#>` cannot land inside a body and make an - # otherwise-empty section look populated. - $body = $Text -replace '^\s*<#', '' -replace '#>\s*$', '' - + # $Text is a RUN's help source, so every `<#`/`#>` delimiter and every line comment's + # leading `#` has already been removed by Get-PfbCommentText. Nothing here has to strip + # them, and nothing here may assume one comment token per help block: a run of adjacent + # blocks is one help block to Get-Help, and its sections have to be read as one stream. $sections = [System.Collections.Generic.List[object]]::new() $current = $null - foreach ($line in ($body -split "\r?\n")) { + foreach ($line in ($Text -split "\r?\n")) { if ($line.Trim() -match '^\.([A-Za-z]+)(?:\s+(\S+))?$') { $current = [PSCustomObject]@{ Keyword = $Matches[1].ToUpperInvariant() @@ -56,82 +84,248 @@ BeforeAll { return $sections } - # Which comment block `Get-Help` actually reads, for a function. + # One comment token as help SOURCE text. + function Get-PfbCommentText { + param( + [System.Management.Automation.Language.Token]$Comment + ) + + $text = $Comment.Text + if ($text -like '<#*') { + # Delimiters removed IN PLACE rather than by dropping the lines that carry them: + # measured, `<# .SYNOPSIS` and `... PARAM-TEXT #>` both keep the content that shares a + # delimiter's line, so dropping those lines would silently lose a section. + $text = $text.Substring(2) + if ($text.EndsWith('#>')) { $text = $text.Substring(0, $text.Length - 2) } + return $text + } + + # A `#` line comment contributes its text with the single leading `#` removed, and that + # text is help source like any other. Measured both consequences: `# .PARAMETER Name` in a + # run really does document Name, and `#region helpers` on the line above a help block + # destroys the block by putting non-directive text first. + return $text.Substring(1) + } + + # Group a region's comment tokens into RUNS: consecutive lines are one run, a blank line starts + # a new one. Verified on both editions that the tokeniser emits one NewLine token per line + # break and never collapses them, so "at most one NewLine between two comments" is exactly "no + # blank line between them". + function Get-PfbCommentRun { + param( + [System.Management.Automation.Language.Token[]]$Comments, + [System.Management.Automation.Language.Token[]]$Tokens + ) + + $newLineKind = [System.Management.Automation.Language.TokenKind]::NewLine + + $runs = [System.Collections.Generic.List[object]]::new() + $current = $null + $previous = $null + foreach ($comment in $Comments) { + $sameRun = $false + if ($null -ne $previous) { + $gap = @($Tokens | Where-Object { + $_.Extent.StartOffset -ge $previous.Extent.EndOffset -and + $_.Extent.EndOffset -le $comment.Extent.StartOffset + }) + $sameRun = (@($gap | Where-Object { $_.Kind -ne $newLineKind }).Count -eq 0) -and + (@($gap | Where-Object { $_.Kind -eq $newLineKind }).Count -le 1) + } + if (-not $sameRun) { + $current = [System.Collections.Generic.List[object]]::new() + $runs.Add($current) + } + $current.Add($comment) + $previous = $comment + } + + # Comma on purpose. Returning the list bare lets the pipeline enumerate it, and a region + # holding exactly one run would then come back as that run's tokens instead of as a list of + # one run -- which reads as "several runs of one comment each" and quietly disables the + # composition test this whole function exists to feed. + return , $runs + } + + # Concatenate one run into the help source Get-Help would see. + function Get-PfbRunText { + param( + [object]$Run + ) + + return ((@($Run) | ForEach-Object { Get-PfbCommentText -Comment $_ }) -join "`n") + } + + # Why a run is not comment-based help -- and the predicate built on it. # - # MEASURED, not assumed. 34 probe shapes were written to disk, dot-sourced, and read back - # through `Get-Help -Full` under BOTH Windows PowerShell 5.1 (5.1.26100.8875) and PowerShell - # 7.6.5. Every shape gave the same answer on both editions, so there is one rule to encode - # rather than one per edition. + # A run stands or falls WHOLE. Get-Help walks its lines in order and abandons all of them the + # moment one is wrong, so a block whose `.SYNOPSIS` and every `.PARAMETER` are perfect renders + # NOTHING if a single line elsewhere in it is a directive Get-Help cannot accept. Two ways a + # line can be wrong, both measured on both editions: + # - a line that is not a directive at all, sitting ABOVE the first directive. BELOW one it is + # simply that section's body, which is why a `#` note under the block is harmless and the + # same note above it is fatal. + # - a directive-SHAPED line -- a dot followed by word characters -- that is not a recognised + # directive, ANYWHERE in the run. `.WIBBLE`, `.SOME_THING`, a bare `.PARAMETER`, + # `.DESCRIPTION with text on the same line`, and (because the pattern is `\w`, not + # `[A-Za-z]`) a prose line such as `.5 is a fraction` each void the entire block. + # A line that starts with a dot but is not word-shaped, such as + # `.\tools\Update-PfbContextHelp.ps1 -WhatIf` in an .EXAMPLE body, is ordinary body text and + # harmless. Measured, not assumed -- the two look alike and behave oppositely. # - # HONOURED positions: - # - immediately ABOVE the `function` keyword, separated by at most one blank line and by - # nothing else. Two blank lines breaks it; so does an intervening `#` line comment. - # - at the START of the body, BEFORE the param block. `[CmdletBinding()]` is part of the - # param block, so a block between the attribute and `param(` is NOT honoured, and neither - # is one immediately BELOW `param(` -- that one is the shape this lookup used to credit. - # - at the END of the body, after the last statement. Trailing blank lines are fine. - # NOT honoured: anywhere in the middle of the body, and anywhere inside a `process { }` or - # other sub-block. `Get-Help` renders its auto-generated syntax-only help instead. + # This is what closes the shapes the position-only version of this lookup failed open on. An + # ordinary `<#…#>` block, or an unknown-keyword one, on the line directly above a help block + # takes the help block down with it; a stray `.WIBBLE` below one does the same. A blank line + # before an offending BLOCK makes them two runs again and the help renders -- but a bad line + # INSIDE the block cannot be rescued that way. # - # PRECEDENCE, when more than one block competes: - # - ABOVE beats every block inside the body, including one at the start of the body. - # - START of body beats END of body. - # - blocks on CONSECUTIVE lines form one run and are concatenated, so a later `.SYNOPSIS` - # overrides an earlier one: the LAST `.SYNOPSIS` in a run wins. - # - a blank line ends a run, and the FIRST run carrying any help keyword is the help. A run - # whose keywords do NOT include `.SYNOPSIS` still wins and suppresses every later block: - # `.DESCRIPTION` alone renders an EMPTY synopsis rather than falling through to a - # `.SYNOPSIS` further down. That is why the keyword pattern below is any `.KEYWORD` and - # not just `.SYNOPSIS` -- matching only `.SYNOPSIS` would credit a block `Get-Help` never - # reads. + # Returns $null when the run IS help, an EMPTY string when it is plain commentary carrying no + # directive at all (a comment, not a defect), and otherwise a reason naming the offending line. + # That third case is the one a developer cannot see by reading the block, so the failure message + # quotes it rather than saying "no help block". + function Get-PfbHelpRunDefect { + param( + [string]$Text + ) + + $hasDirective = $Text -match '(?m)^\s*\.\w+' + $seenDirective = $false + + foreach ($line in ($Text -split "\r?\n")) { + if ($line -match '^\s*$') { continue } + + # Get-Help's own directive pattern, verbatim -- both editions' comment parser carries + # this exact expression, argument group and all. + if ($line -notmatch '^\s*\.(\w+)(\s+(\S.*))?\s*$') { + if ($seenDirective) { continue } + if ($hasDirective) { + return "a line that is not a help directive sits above the first one: '$($line.Trim())'" + } + return '' + } + + $keyword = $Matches[1].ToUpperInvariant() + $argument = $Matches[3] + + if ($script:helpDirectiveBare -contains $keyword) { + if ([string]::IsNullOrWhiteSpace($argument)) { $seenDirective = $true; continue } + return ".$keyword takes no argument, so '$($line.Trim())' is not a directive line" + } + if ($script:helpDirectiveArgument -contains $keyword) { + if (-not [string]::IsNullOrWhiteSpace($argument)) { $seenDirective = $true; continue } + return ".$keyword needs an argument, so '$($line.Trim())' is not a directive line" + } + return ".$keyword is not a directive Get-Help recognises, and one bad directive voids the whole block" + } + + if ($seenDirective) { return $null } + return '' + } + + function Test-PfbHelpRun { + param( + [string]$Text + ) + + return ($null -eq (Get-PfbHelpRunDefect -Text $Text)) + } + + # The first run in ONE region that is help, as source text; $null when the region holds none. # - # A `#` line comment inside a run cuts both ways, asymmetrically, and both directions were - # measured: one BEFORE the block comment makes `Get-Help` render nothing, while one AFTER it - # is honoured with the line's text appended to whatever section came last. Public/ contains - # the second shape, so the two cannot be collapsed. + # A run that is help claims it absolutely -- whether or not it carries a `.SYNOPSIS` this sweep + # can score -- which is what stops the end-of-body search crediting a block Get-Help never + # reaches. A run that is NOT help claims nothing and blocks nothing. + function Select-PfbHelpFromRegion { + param( + [System.Management.Automation.Language.Token[]]$Comments, + [System.Management.Automation.Language.Token[]]$Tokens + ) + + $defect = $null + foreach ($run in (Get-PfbCommentRun -Comments $Comments -Tokens $Tokens)) { + $text = Get-PfbRunText -Run $run + $why = Get-PfbHelpRunDefect -Text $text + if ($null -eq $why) { return [PSCustomObject]@{ Text = $text; Defect = $null } } + # First malformed help block in the region wins the diagnosis. Plain commentary + # reports an empty string and is not a defect. + if ($why -and -not $defect) { $defect = $why } + } + + return [PSCustomObject]@{ Text = $null; Defect = $defect } + } + + # Which comment run `Get-Help` actually renders, for a function. + # + # MEASURED, not assumed, and not taken from any prose account -- this file's own comments + # included, two of which were wrong. 125 probe shapes were written to disk, dot-sourced and read + # back through `Get-Help -Full` under BOTH Windows PowerShell 5.1 (5.1.26100.8875) and + # PowerShell 7.6.5, and then all 41 fixtures below were put through the same treatment and + # compared against this detector. Every shape gave the same placement answer on both editions, + # so there is one rule to encode rather than one per edition. + # + # The unit is the RUN, defined by Get-PfbCommentRun above, not the block. Modelling position + # alone is precisely what made the previous version fail open: it asked where a `<#…#>` block + # sat and never asked what shared its run. # - # Two known places where this is deliberately narrower than `Get-Help`, both erring toward a - # red build rather than a false green: - # - line-comment-style help (`# .SYNOPSIS` on consecutive lines) is not recognised at all, - # above or inside. It is legal PowerShell and nothing in Public/ uses it. - # - a run whose only help keyword is one `Get-Help` does not recognise (`.FOO`) is treated as - # claiming the help, so nothing later is credited. `Get-Help` may well fall through to a - # later block; the cost of being wrong here is a red build on a shape nobody writes. - function Get-PfbHelpToken { + # REGIONS, searched in this order and moving on whenever a region holds no help run: + # 1. ABOVE the `function` keyword -- the LAST run before it, and only when nothing but at + # most one blank line separates the two. Two blank lines breaks it, and so does another + # comment run in between, which is why a file header with unrelated commentary beneath it + # attaches to nothing. An intervening `#` line comment does NOT break it: the comment + # simply joins the run, and its text is appended to the last section. + # 2. START of the body -- the runs before the first code token. `[CmdletBinding()]` is code, + # so a run between the attribute and `param(` is already past this region, as is one below + # `param()`. There is no proximity rule inside the body; blank lines either side are fine. + # 3. END of the body -- the runs after the last code token. + # Nothing else is honoured. Mid-body, or inside a `process { }` or other sub-block, renders the + # auto-generated syntax help. + # + # PRECEDENCE follows from the region order: above beats the body, and start of body beats end of + # body. A run that is help but carries no `.SYNOPSIS` still claims it -- `.DESCRIPTION` alone + # renders an EMPTY synopsis rather than falling through to a `.SYNOPSIS` further down. + # + # One deliberate narrowing, erring toward a red build rather than a false green: an ABOVE run + # that Get-Help does render is reported rather than credited. See Get-PfbHelpRecord for why. + function Get-PfbRenderedHelp { param( [System.Management.Automation.Language.FunctionDefinitionAst]$Function, - [System.Management.Automation.Language.Token[]]$Tokens, - [string]$KeywordPattern, - [string]$SynopsisPattern + [System.Management.Automation.Language.Token[]]$Tokens ) $commentKind = [System.Management.Automation.Language.TokenKind]::Comment $newLineKind = [System.Management.Automation.Language.TokenKind]::NewLine - # ABOVE the function keyword. Reported, not credited -- see the record's - # HelpAboveFunction field for why that is a finding rather than a pass. - foreach ($token in $Tokens) { - if ($token.Kind -ne $commentKind) { continue } - if ($token.Text -notlike '<#*') { continue } - if ($token.Text -notmatch $KeywordPattern) { continue } - if ($token.Extent.EndOffset -gt $Function.Extent.StartOffset) { continue } - + # Region 1 -- ABOVE the function keyword. + $above = @($Tokens | Where-Object { + $_.Kind -eq $commentKind -and + $_.Extent.EndOffset -le $Function.Extent.StartOffset + }) + $aboveRuns = Get-PfbCommentRun -Comments $above -Tokens $Tokens + if ($aboveRuns.Count -gt 0) { + $lastRun = $aboveRuns[$aboveRuns.Count - 1] + $lastComment = $lastRun[$lastRun.Count - 1] $between = @($Tokens | Where-Object { - $_.Extent.StartOffset -ge $token.Extent.EndOffset -and + $_.Extent.StartOffset -ge $lastComment.Extent.EndOffset -and $_.Extent.EndOffset -le $Function.Extent.StartOffset }) - if (@($between | Where-Object { $_.Kind -ne $newLineKind }).Count -gt 0) { continue } # One newline is adjacency, two is a single blank line -- both honoured. Three is two # blank lines, which is not. - if ($between.Count -gt 2) { continue } + $adjacent = (@($between | Where-Object { $_.Kind -ne $newLineKind }).Count -eq 0) -and + ($between.Count -le 2) - return [PSCustomObject]@{ Token = $null; AboveFunction = $true } + if ($adjacent -and (Test-PfbHelpRun -Text (Get-PfbRunText -Run $lastRun))) { + return [PSCustomObject]@{ Text = $null; AboveFunction = $true; Defect = $null } + } + # Not adjacent, or adjacent but not help: measured, Get-Help then reads the body + # exactly as though nothing sat above the function at all. Falling through rather than + # stopping here is what keeps a `#region` marker or a file header from being read as a + # cmdlet having no help. } - # Inside the body. Work from the TOKENS rather than the statement list: the param block, - # its `[CmdletBinding()]` attribute and a named `begin`/`process`/`end` block are all - # "code" for this purpose, and the token stream treats them uniformly. Body.Extent - # includes the braces, so the strict comparisons drop them. + # Regions 2 and 3 -- inside the body. Work from the TOKENS rather than the statement list: + # the param block, its `[CmdletBinding()]` attribute and a named `begin`/`process`/`end` + # block are all "code" for this purpose, and the token stream treats them uniformly. + # Body.Extent includes the braces, so the strict comparisons drop them. $bodyStart = $Function.Body.Extent.StartOffset $bodyEnd = $Function.Body.Extent.EndOffset $inBody = @($Tokens | Where-Object { @@ -152,86 +346,13 @@ BeforeAll { $leading = @($comments | Where-Object { $_.Extent.EndOffset -le $codeStart }) $trailing = @($comments | Where-Object { $_.Extent.StartOffset -ge $codeEnd }) - $found = Select-PfbHelpFromRegion -Comments $leading -Tokens $Tokens ` - -KeywordPattern $KeywordPattern -SynopsisPattern $SynopsisPattern - if (-not $found.Stopped) { - $found = Select-PfbHelpFromRegion -Comments $trailing -Tokens $Tokens ` - -KeywordPattern $KeywordPattern -SynopsisPattern $SynopsisPattern + $found = Select-PfbHelpFromRegion -Comments $leading -Tokens $Tokens + if ($null -eq $found.Text) { + $end = Select-PfbHelpFromRegion -Comments $trailing -Tokens $Tokens + if ($null -ne $end.Text -or $null -eq $found.Defect) { $found = $end } } - return [PSCustomObject]@{ Token = $found.Token; AboveFunction = $false } - } - - # Pick the block `Get-Help` would read out of ONE honoured region (start of body, or end of - # body), per the precedence measured above. - # - # `Stopped` says the region held a help block, whether or not that block turned out to carry a - # `.SYNOPSIS` this sweep can score. It is what keeps the end-of-body search from crediting a - # block `Get-Help` never reaches, because a start-of-body block already claimed the help. - function Select-PfbHelpFromRegion { - param( - [System.Management.Automation.Language.Token[]]$Comments, - [System.Management.Automation.Language.Token[]]$Tokens, - [string]$KeywordPattern, - [string]$SynopsisPattern - ) - - $newLineKind = [System.Management.Automation.Language.TokenKind]::NewLine - - # Group into runs: consecutive lines are one run, a blank line starts a new one. - $runs = [System.Collections.Generic.List[object]]::new() - $current = $null - $previous = $null - foreach ($comment in $Comments) { - $sameRun = $false - if ($null -ne $previous) { - $gap = @($Tokens | Where-Object { - $_.Extent.StartOffset -ge $previous.Extent.EndOffset -and - $_.Extent.EndOffset -le $comment.Extent.StartOffset - }) - $sameRun = (@($gap | Where-Object { $_.Kind -ne $newLineKind }).Count -eq 0) -and - (@($gap | Where-Object { $_.Kind -eq $newLineKind }).Count -le 1) - } - if (-not $sameRun) { - $current = [System.Collections.Generic.List[object]]::new() - $runs.Add($current) - } - $current.Add($comment) - $previous = $comment - } - - foreach ($run in $runs) { - # A run of ordinary commentary is not help and does not suppress what follows it -- - # measured: a line comment, a blank line, then the real block, and Get-Help reads the - # block. So the keyword test comes FIRST, before the composition test below. - $keyworded = @($run | Where-Object { $_.Text -like '<#*' -and $_.Text -match $KeywordPattern }) - if ($keyworded.Count -eq 0) { continue } - - # A run carrying help keywords claims the help, so from here on every path stops. - # - # Position of a `#` line comment inside the run decides it, and the two directions are - # not symmetric -- measured, on both editions. A line comment BEFORE the block makes - # Get-Help render nothing at all. One AFTER the block is honoured, with the line's text - # appended to whichever section came last. Public/Get-PfbOpenFile.ps1 is the second - # shape (a drift-report note between the block and [CmdletBinding()]), so treating the - # two alike would red-build a cmdlet whose help renders correctly. - $firstKeyworded = $keyworded[0] - $precedingLineComment = @($run | Where-Object { - $_.Text -notlike '<#*' -and - $_.Extent.StartOffset -lt $firstKeyworded.Extent.StartOffset - }) - if ($precedingLineComment.Count -gt 0) { - return [PSCustomObject]@{ Token = $null; Stopped = $true } - } - - $synopsis = @($run | Where-Object { $_.Text -like '<#*' -and $_.Text -match $SynopsisPattern }) - if ($synopsis.Count -eq 0) { - return [PSCustomObject]@{ Token = $null; Stopped = $true } - } - return [PSCustomObject]@{ Token = $synopsis[$synopsis.Count - 1]; Stopped = $true } - } - - return [PSCustomObject]@{ Token = $null; Stopped = $false } + return [PSCustomObject]@{ Text = $found.Text; AboveFunction = $false; Defect = $found.Defect } } # Reduce one cmdlet to the facts the assertions below need. @@ -248,16 +369,15 @@ BeforeAll { $declared = @($paramBlock.Parameters | ForEach-Object { $_.Name.VariablePath.UserPath }) } - # Locate the help block from the TOKEN stream, not by regexing the file: a comment token is - # the only thing that is definitely a comment, and the AST has no node for one. + # Locate the help from the TOKEN stream, not by regexing the file: a comment token is the + # only thing that is definitely a comment, and the AST has no node for one. # - # A `.SYNOPSIS` keyword line is the marker (anchored, per Get-PfbHelpSection's reasoning), - # so a .DESCRIPTION or .EXAMPLE that merely mentions the word cannot be mistaken for the - # block. - # - # The block must sit at a position `Get-Help` HONOURS, and it must be inside the function - # body. Get-PfbHelpToken above carries the measured placement and precedence rules; two - # points about how this gate uses them: + # Get-PfbRenderedHelp above carries the measured placement, composition and precedence + # rules, and returns the help SOURCE -- the whole claiming run concatenated -- rather than + # one token. That matters here: a run of adjacent blocks is a single help block to + # Get-Help, so every `.PARAMETER` in it counts no matter which block carried it. Scoring + # one token instead hid the other blocks' entries from the orphan and duplicate + # assertions ('two-blocks-one-run' pins it). Three points about how this gate uses the rest: # # Being inside the function extent is not enough, which is what this lookup used to check. # A block below `param()`, or in the middle of a body after an early-return guard, is @@ -265,6 +385,11 @@ BeforeAll { # so the old test reported "fully documented" for a cmdlet with no rendered help at all. # The 'help-below-param' and 'help-mid-body' fixtures pin that. # + # Neither is POSITION enough on its own, which is what the version before this one checked. + # A block on the line directly below ordinary commentary is at an honoured position and + # renders nothing, because the run it belongs to does not open with a directive + # ('ordinary-then-help-one-run', 'unknown-keyword-then-help-one-run', 'prose-before-keyword'). + # # A block immediately ABOVE the `function` keyword is honoured by `Get-Help` -- that much # the earlier comment here had backwards -- but it is still a finding, and the convention # is a block inside the body (all 544 in Public/ are). Two reasons to keep flagging it. @@ -274,16 +399,15 @@ BeforeAll { # with Get-Help about adjacency to the byte, where the failure direction is a file-header # block getting credited as a cmdlet's help ('distant-header'). Reported separately, via # HelpAboveFunction, so the failure message can name the actual remedy. - # `\r` is in the trailing character class on purpose. In multiline mode `$` matches before + $lookup = Get-PfbRenderedHelp -Function $Function -Tokens $Tokens + $helpText = $lookup.Text + + # Used for DIAGNOSIS only, below. Case-insensitive because Get-Help's directives are, and + # `\r` is in the trailing character class on purpose: in multiline mode `$` matches before # the `\n` but NOT before the `\r`, so a `[ \t]*$` tail silently matches nothing at all in # a CRLF file -- which every file in this repo is. The symptom is the whole population # reading as undocumented while the same pattern works on an LF fixture. - $keywordPattern = '(?m)^[ \t]*\.[A-Za-z]+(?:[ \t]+\S+)?[ \t\r]*$' - $synopsisPattern = '(?m)^[ \t]*\.SYNOPSIS[ \t\r]*$' - - $lookup = Get-PfbHelpToken -Function $Function -Tokens $Tokens ` - -KeywordPattern $keywordPattern -SynopsisPattern $synopsisPattern - $help = $lookup.Token + $synopsisPattern = '(?im)^[ \t]*\.SYNOPSIS[ \t\r]*$' # Nested helpers, for DIAGNOSIS only. The position test above already rejects a nested # helper's block (it can be neither before the outer body's first code token nor after its @@ -306,42 +430,41 @@ BeforeAll { $candidate.Extent.EndOffset -le $_.Extent.EndOffset }).Count }) - $misplaced = ($null -eq $help) -and (-not $lookup.AboveFunction) -and ($ownCandidates.Count -gt 0) - $synopsisEmpty = $false $seenSynopsis = $false $documented = @() $emptyParameterSections = @() - $namelessParameterSections = 0 - if ($null -ne $help) { - $sections = Get-PfbHelpSection -Text $help.Text + if ($null -ne $helpText) { + $sections = @(Get-PfbHelpSection -Text $helpText) + + # The LAST .SYNOPSIS wins, and this is the reverse of what this file used to assert. + # Measured on both editions, within one block and across blocks of one run alike: a + # later .SYNOPSIS overrides an earlier one even when it is EMPTY, and Get-Help then + # renders a blank synopsis rather than the earlier text. Taking the first was the false + # green -- it reported a populated synopsis for a cmdlet whose rendered synopsis is + # blank, which is exactly what this assertion exists to catch. + $synopsisSections = @($sections | Where-Object { $_.Keyword -eq 'SYNOPSIS' }) + if ($synopsisSections.Count -gt 0) { + $seenSynopsis = $true + $lastSynopsis = $synopsisSections[$synopsisSections.Count - 1] + $synopsisEmpty = [string]::IsNullOrWhiteSpace((($lastSynopsis.BodyLines -join "`n").Trim())) + } foreach ($section in $sections) { - $sectionBody = ($section.BodyLines -join "`n").Trim() - - if ($section.Keyword -eq 'SYNOPSIS') { - # First .SYNOPSIS wins -- enforced, not merely described. The previous guard - # was `-not $synopsisEmpty`, which only prevented un-setting a flag that is - # never un-set: given a populated first .SYNOPSIS and an empty second, the loop - # reached the second and flagged the cmdlet. Wrong direction is a false - # positive rather than a false green, but the comment claimed a behaviour the - # code did not have, which is the defect class 3c98a7f already paid for. - if (-not $seenSynopsis) { - $seenSynopsis = $true - $synopsisEmpty = [string]::IsNullOrWhiteSpace($sectionBody) - } - continue - } - if ($section.Keyword -ne 'PARAMETER') { continue } - if ([string]::IsNullOrEmpty($section.Argument)) { - # `.PARAMETER` with no name documents nothing and names nothing, so neither the - # coverage nor the orphan assertion would see it. Counted separately. - $namelessParameterSections++ - continue - } + $sectionBody = ($section.BodyLines -join "`n").Trim() + + # A NAMELESS `.PARAMETER` cannot reach here and is deliberately not counted. It + # used to have a field and an assertion of its own, on the belief that Get-Help + # renders the rest of the block and silently drops the entry. Measured on both + # editions, it does not: a bare `.PARAMETER` is a malformed directive and voids + # the whole block, so the cmdlet has no rendered help at all. That is caught + # earlier and harder, by HasHelpBlock, and named exactly by HelpRunDefect -- so + # the counter could never have been non-zero once the run test was correct, and a + # field that can only ever read 0 is an assertion that cannot fail. + if ([string]::IsNullOrEmpty($section.Argument)) { continue } $documented += $section.Argument if ([string]::IsNullOrWhiteSpace($sectionBody)) { @@ -350,6 +473,8 @@ BeforeAll { } } + $misplaced = (-not $seenSynopsis) -and (-not $lookup.AboveFunction) -and ($ownCandidates.Count -gt 0) + # Case-insensitive both ways: PowerShell parameter names are case-insensitive, so a # `.PARAMETER filter` documenting `[string]$Filter` is correct help and must not read as a # miss in one direction and an orphan in the other. @@ -368,9 +493,10 @@ BeforeAll { File = $File Function = $Function.Name Line = $Function.Extent.StartLineNumber - HasHelpBlock = ($null -ne $help) + HasHelpBlock = $seenSynopsis HelpAboveFunction = $lookup.AboveFunction HelpBlockMisplaced = $misplaced + HelpRunDefect = $lookup.Defect SynopsisEmpty = $synopsisEmpty DeclaredParameters = $declared DocumentedParameters = $documented @@ -378,7 +504,6 @@ BeforeAll { OrphanedParameters = $orphaned DuplicatedParameters = $duplicated EmptyParameterSections = $emptyParameterSections - NamelessParameterSections = $namelessParameterSections } } @@ -446,23 +571,37 @@ Describe 'Comment-based help coverage' { # them would let a bare `.SYNOPSIS` line satisfy a presence check while documenting nothing. # # PLACEMENT is part of this assertion, so the message has to name it: a cmdlet can hold a - # perfectly good .SYNOPSIS and still fail here because it sits where `Get-Help` will not - # read it. Each offender is annotated with which of the three it is, because the remedy - # differs -- write the help, or move it. + # perfectly good .SYNOPSIS and still fail here because Get-Help will not read it there. + # Each offender is annotated with which of the three it is, because the remedy differs -- + # write the help, or move it. + # + # The above-the-function wording is deliberate and was wrong until 2026-08-25. It used to + # tell the developer that Get-Help does not render help above the function. It DOES -- + # measured on both editions. The finding is a convention one: an outer block renders IN + # PLACE OF a block inside the body, so a cmdlet carrying both has inner help no reader ever + # sees, and crediting the outer block would force this sweep to agree with Get-Help about + # adjacency to the byte, whose failure direction is a file header scored as a cmdlet's help. + # A message that misstates the reason sends the reader looking for a rendering bug that is + # not there. $missingBlock = @($script:cmdlets | Where-Object { -not $_.HasHelpBlock }) $missingDetail = @($missingBlock | ForEach-Object { $reason = if ($_.HelpAboveFunction) { - 'help block sits ABOVE the function keyword -- move it inside the body' + 'help block sits ABOVE the function keyword. Get-Help DOES render it there, so this is a convention finding, not a broken-help one -- but it renders INSTEAD OF any block inside the body, and the convention throughout Public/ is a block inside the body; move it inside' + } + elseif ($_.HelpRunDefect) { + # The one case a developer cannot diagnose by reading the block, because the + # block looks correct: one bad line voids all of it. Quote the line. + "the help block is malformed and Get-Help renders NONE of it -- $($_.HelpRunDefect)" } elseif ($_.HelpBlockMisplaced) { - 'help block is at a position Get-Help does not honour (mid-body, or below param()) -- move it to the top of the body' + 'the cmdlet has a .SYNOPSIS that Get-Help does not render -- either it sits where Get-Help never looks (below param(), mid-body, or inside a process block), or it shares a comment run with a line that is not a help directive, which voids the whole run; move it to the top of the body and leave a blank line between it and any comment above it' } else { 'no help block at all' } "$($_.File): $($_.Function) -- $reason" }) -join "`n" - $missingDetail | Should -BeNullOrEmpty -Because "every public cmdlet needs comment-based help that Get-Help will actually render: one block comment as the FIRST thing in the function body, above [CmdletBinding()] and param(). A .SYNOPSIS below param(), in the middle of the body, or above the function keyword does not satisfy this even though it is real help text -- move the existing block rather than writing a second one; offenders:`n$missingDetail" + $missingDetail | Should -BeNullOrEmpty -Because "every public cmdlet needs comment-based help that Get-Help will actually render: one block comment as the FIRST thing in the function body, above [CmdletBinding()] and param(), and not sharing a comment run with ordinary commentary. Real help text is not enough on its own -- a .SYNOPSIS below param(), in the middle of the body, or on the line directly below an unrelated comment renders nothing at all. Move the existing block rather than writing a second one; offenders:`n$missingDetail" $emptySynopsis = @($script:cmdlets | Where-Object { $_.SynopsisEmpty }) $emptyDetail = @($emptySynopsis | ForEach-Object { "$($_.File): $($_.Function)" }) -join "`n" @@ -498,11 +637,16 @@ Describe 'Comment-based help coverage' { }) -join "`n" $duplicateDetail | Should -BeNullOrEmpty -Because "a duplicated .PARAMETER entry means one of the two is stale; offenders:`n$duplicateDetail" - $nameless = @($script:cmdlets | Where-Object { $_.NamelessParameterSections -gt 0 }) - $namelessDetail = @($nameless | ForEach-Object { - "$($_.File): $($_.Function) -- $($_.NamelessParameterSections) nameless .PARAMETER" - }) -join "`n" - $namelessDetail | Should -BeNullOrEmpty -Because "a .PARAMETER with no name documents nothing and is invisible to both coverage and orphan checks; offenders:`n$namelessDetail" + # There WAS a third assertion here, for a `.PARAMETER` with no name. It is gone, and this + # note is the record of why rather than a silent deletion. It rested on the belief that + # Get-Help renders the rest of such a block and quietly drops the nameless entry, which + # would make it invisible to both assertions above. Measured on both editions, that is not + # what happens: a bare `.PARAMETER` is a malformed directive, and one malformed directive + # voids the entire block, so the cmdlet renders no help at all. The first assertion in this + # file therefore catches it, earlier and harder, and names the offending line via + # HelpRunDefect. The counter it read could no longer be non-zero, and an assertion over a + # field that is structurally always 0 is one that cannot fail -- which is the failure mode + # the anti-vacuous floor at the top of this file exists to prevent. } It 'gives every .PARAMETER entry a body' { @@ -891,6 +1035,277 @@ function Get-PfbFixture { [CmdletBinding()] param([Parameter()] [string]$Name) } +'@ + # RUN COMPOSITION. The last group, and the one the position-only version of this lookup + # had no model for at all: which comment run a block belongs to decides whether it is + # help, independently of where the run sits. Each of these was measured against real + # `Get-Help -Full` output on both editions, and each is paired with the same shape one + # blank line apart -- the pair is the point, because the two differ by a single + # character of whitespace and Get-Help answers them oppositely. + 'above-linecomment-then-inside' = @' +<# +.SYNOPSIS + Adjacent help above the function, with a note between it and the keyword. +.PARAMETER Other + A parameter this cmdlet does not declare. +#> +# A note between the help block and the function keyword. +function Get-PfbFixture { + <# + .SYNOPSIS + Help inside the body, which Get-Help never renders because of the run above. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'ordinary-then-help-one-run' = @' +function Get-PfbFixture { + <# + An ordinary comment block on the line directly above the help block. + #> + <# + .SYNOPSIS + Help in a run that does not open with a directive, so Get-Help reads none of it. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'ordinary-blank-then-help' = @' +function Get-PfbFixture { + <# + An ordinary comment block, one blank line above the help block. + #> + + <# + .SYNOPSIS + Help in a run of its own, which Get-Help renders normally. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'unknown-keyword-then-help-one-run' = @' +function Get-PfbFixture { + <# + .WIBBLE + A keyword Get-Help does not recognise, on the line above the help block. + #> + <# + .SYNOPSIS + Help in the same run, which Get-Help therefore never reads. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'unknown-keyword-blank-then-help' = @' +function Get-PfbFixture { + <# + .WIBBLE + A keyword Get-Help does not recognise, one blank line above the help block. + #> + + <# + .SYNOPSIS + Help in a run of its own, which Get-Help renders. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'prose-before-keyword' = @' +function Get-PfbFixture { + <# + Prose above the first directive, inside the block rather than above it. + .SYNOPSIS + Help Get-Help refuses to read, for the same reason as the two shapes above. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'synopsis-with-inline-argument' = @' +function Get-PfbFixture { + <# + .SYNOPSIS Help text on the directive line, which makes it not a directive line. + .PARAMETER Other + A parameter this cmdlet does not declare. + #> + + <# + .SYNOPSIS + The block Get-Help actually renders. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'linecomment-help-claims-run' = @' +function Get-PfbFixture { + # .SYNOPSIS + # Line-comment-style help, which Get-Help does render. + # .PARAMETER Other + # A parameter this cmdlet does not declare. + + <# + .SYNOPSIS + A block Get-Help never reaches, because the run above already claimed the help. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'linecomment-continues-run' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + Help whose run continues into the line comments below it. + #> + # .PARAMETER Name + # The fixture name, documented by a line comment in the same run. + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'invalid-above-falls-through' = @' +<# + An ordinary block sharing a run with the help block above the function. +#> +<# +.SYNOPSIS + Help that looks adjacent but whose run does not open with a directive. +.PARAMETER Other + A parameter this cmdlet does not declare. +#> +function Get-PfbFixture { + <# + .SYNOPSIS + The block Get-Help renders, because nothing above the function is help. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + # Content sharing a line with a delimiter, at both ends. Measured: Get-Help keeps it, + # so the delimiters have to be stripped IN PLACE. Dropping the `<#` line instead loses + # the .SYNOPSIS and voids the block; dropping the `#>` line silently empties the last + # section. Neither is visible to any other fixture here, because every other fixture + # puts its delimiters on lines of their own. + 'delimiter-line-content' = @' +function Get-PfbFixture { + <# .SYNOPSIS + A synopsis whose directive shares the opening delimiter's line. + .PARAMETER Name + The fixture name, ending on the closing delimiter's line. #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + # A malformed directive LATER in an otherwise-perfect block. The run test has to walk + # every line, not just the first: these two blocks open correctly, document their + # parameter correctly, and render nothing at all. + 'unknown-keyword-later-in-block' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + A synopsis Get-Help never renders, because of the line four below it. + .PARAMETER Name + The fixture name. + .WIBBLE + A keyword Get-Help does not recognise, after otherwise-correct help. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'dotword-prose-voids-block' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + A synopsis Get-Help never renders, because of the .EXAMPLE body below. + .PARAMETER Name + The fixture name. + .EXAMPLE + .NET Core is mentioned at the start of this line + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + # The adjacency BOUNDARY above the function, both sides of it. 'distant-header' never + # reaches this: its last run before the keyword is the line comment, so the block above + # it never gets an adjacency test at all, and the bound went unpinned until a mutation + # of it survived. + 'above-blank-line-still-adjacent' = @' +<# +.SYNOPSIS + A block one blank line above the function, which Get-Help still attaches to it. +.PARAMETER Name + The fixture name. +#> + +function Get-PfbFixture { + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'distant-block-falls-through' = @' +<# +.SYNOPSIS + A block two blank lines above the function, which Get-Help does not attach to it. +.PARAMETER Other + A parameter this cmdlet does not declare. +#> + + +function Get-PfbFixture { + <# + .SYNOPSIS + The block Get-Help renders, because two blank lines break adjacency above. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} +'@ + 'ordinary-start-help-at-end' = @' +function Get-PfbFixture { + <# + An ordinary block at the start of the body, which is not help. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) + + Write-Output 'body' + + <# + .SYNOPSIS + Help at the end of the body, which Get-Help falls through to. + .PARAMETER Name + The fixture name. + #> +} '@ } @@ -919,7 +1334,7 @@ function Get-PfbFixture { $records['clean'].OrphanedParameters | Should -BeNullOrEmpty $records['clean'].DuplicatedParameters | Should -BeNullOrEmpty $records['clean'].EmptyParameterSections | Should -BeNullOrEmpty - $records['clean'].NamelessParameterSections | Should -Be 0 + $records['clean'].HelpRunDefect | Should -BeNullOrEmpty $records['no-help'].HasHelpBlock | Should -BeFalse $records['empty-synopsis'].HasHelpBlock | Should -BeTrue @@ -941,9 +1356,18 @@ function Get-PfbFixture { Should -BeNullOrEmpty -Because 'the entry exists, so it is an empty-body finding and not a coverage finding' $records['duplicate'].DuplicatedParameters | Should -Be @('Name') - $records['nameless'].NamelessParameterSections | Should -Be 1 + + # A bare `.PARAMETER` does NOT merely fail to document its parameter. Measured on both + # editions: it is a malformed directive, and one malformed directive voids the whole block, + # so Get-Help renders auto-generated syntax help and the .SYNOPSIS above it never appears + # either. This fixture asserted a nameless-section COUNT until 2026-08-25, on the opposite + # belief -- crediting the block was a false green over a cmdlet with no rendered help. + $records['nameless'].HasHelpBlock | + Should -BeFalse -Because 'measured: a bare .PARAMETER voids the entire block, .SYNOPSIS included' $records['nameless'].MissingParameters | - Should -Be @('Name') -Because 'a nameless entry documents nothing, so the parameter is still undocumented' + Should -Be @('Name') -Because 'nothing in a voided block is credited' + $records['nameless'].HelpRunDefect | + Should -BeLike '*PARAMETER needs an argument*' -Because 'the failure message has to name the line that voided the block, or the developer is left reading a block that looks perfectly correct' # False-positive guards. $records['dotted-prose'].MissingParameters | Should -BeNullOrEmpty @@ -1023,7 +1447,8 @@ function Get-PfbFixture { $records['two-blocks-one-run'].HasHelpBlock | Should -BeTrue $records['two-blocks-one-run'].MissingParameters | Should -BeNullOrEmpty -Because 'blocks on consecutive lines are one run and a later .SYNOPSIS overrides an earlier one, so the SECOND is what Get-Help renders' - $records['two-blocks-one-run'].OrphanedParameters | Should -BeNullOrEmpty + $records['two-blocks-one-run'].OrphanedParameters | + Should -Be @('Other') -Because 'a run is ONE help block to Get-Help: the first block''s .PARAMETER Other renders too, so it is a genuine orphan. This expectation was empty until 2026-08-25, when the run was measured rather than described -- scoring only the block that carried the winning .SYNOPSIS hid every other block''s entries from this assertion' $records['two-runs-blank-separated'].HasHelpBlock | Should -BeTrue $records['two-runs-blank-separated'].MissingParameters | @@ -1045,7 +1470,137 @@ function Get-PfbFixture { $records['nested-only'].MissingParameters | Should -Be @('Name') -Because "the nested helper's .PARAMETER entry must not be credited to the cmdlet" + # The LAST .SYNOPSIS wins, not the first. This assertion read -BeFalse until 2026-08-25 on + # the strength of a source comment; measured on both editions, within one block and across + # blocks of one run alike, a later .SYNOPSIS overrides an earlier one EVEN WHEN EMPTY and + # Get-Help renders a blank synopsis. Believing the first won was a false green: it reported + # a populated synopsis for a cmdlet whose rendered synopsis is empty, which is verbatim the + # defect the empty-synopsis assertion exists to catch. $records['second-synopsis'].SynopsisEmpty | - Should -BeFalse -Because 'the first .SYNOPSIS is populated and wins; a later empty one must not flag the cmdlet' + Should -BeTrue -Because 'measured: a trailing empty .SYNOPSIS overrides the populated one above it and Get-Help renders a blank synopsis' + $records['second-synopsis'].HasHelpBlock | + Should -BeTrue -Because 'the run is help and does carry a .SYNOPSIS; the finding is that it renders empty, which is the other assertion' + + # RUN COMPOSITION. Three shapes the position-only lookup scored as documented while + # Get-Help rendered auto-generated syntax help, each paired with the same shape one blank + # line apart to keep the rule from collapsing into "reject any mixed run". + + # HIGH 1. Measured: an intervening `#` line comment does NOT break adjacency above the + # function -- the comment joins the run and its text is appended to the last section, and + # the outer block renders IN PLACE OF the inner one. The old lookup rejected any non-newline + # token between block and keyword, so it ignored the outer block and credited the inner one: + # rendered help and scored help came from different blocks. + $records['above-linecomment-then-inside'].HelpAboveFunction | + Should -BeTrue -Because 'measured on both editions: a line comment between the block and the function keyword is honoured, so this is the above-the-function shape' + $records['above-linecomment-then-inside'].HasHelpBlock | + Should -BeFalse -Because 'Get-Help renders the run above the function, so the block inside the body is dead text' + $records['above-linecomment-then-inside'].MissingParameters | + Should -Be @('Name') -Because 'the inner block must not be credited for a parameter whose help never renders' + + # HIGH 2. An ordinary block on the line above the help block puts non-directive text first + # in the run, and Get-Help reads none of it. + $records['ordinary-then-help-one-run'].HasHelpBlock | + Should -BeFalse -Because 'measured: a run whose first non-blank line is not a directive is ordinary commentary, help block and all, and Get-Help falls back to auto-generated syntax help' + $records['ordinary-then-help-one-run'].MissingParameters | + Should -Be @('Name') -Because 'nothing in a voided run is credited, including its .PARAMETER entries' + $records['ordinary-blank-then-help'].HasHelpBlock | + Should -BeTrue -Because 'measured: one blank line makes them two runs, the second opens with a directive, and Get-Help renders it -- the pair is what stops the rule becoming "reject any mixed run"' + $records['ordinary-blank-then-help'].MissingParameters | Should -BeNullOrEmpty + + # HIGH 3. Same shape with an unrecognised keyword. Both directions matter: same-run voids + # the help block, blank-separated does NOT suppress it. The blank-separated expectation is + # the reverse of what this file used to encode -- it treated any `.KEYWORD` run as claiming + # the help, which red-built a shape Get-Help renders perfectly well. + $records['unknown-keyword-then-help-one-run'].HasHelpBlock | + Should -BeFalse -Because 'measured: .WIBBLE is not a directive Get-Help recognises, so it is prose, and prose first in a run voids the whole run' + $records['unknown-keyword-then-help-one-run'].MissingParameters | Should -Be @('Name') + $records['unknown-keyword-blank-then-help'].HasHelpBlock | + Should -BeTrue -Because 'measured: an unrecognised keyword claims nothing, so a blank-separated help block below it renders normally' + $records['unknown-keyword-blank-then-help'].MissingParameters | Should -BeNullOrEmpty + + # The same defect inside a SINGLE block, which is why the test is "first non-blank line of + # the run" and not "the run's first comment is a block comment". + $records['prose-before-keyword'].HasHelpBlock | + Should -BeFalse -Because 'measured: prose above the first directive voids the block, exactly as an ordinary block on the line above would' + $records['prose-before-keyword'].MissingParameters | Should -Be @('Name') + + # ARITY. `.SYNOPSIS` takes no argument, so `.SYNOPSIS ` is not a directive line at all + # and the block opening with it is prose -- which means the block BELOW it is the help. + # Getting arity wrong in this direction is a false green: it would treat the first run as + # claiming and score nothing. + $records['synopsis-with-inline-argument'].HasHelpBlock | + Should -BeTrue -Because 'measured: an argument-less directive carrying an argument is not a directive line, so that run is prose and Get-Help falls through to the next one' + $records['synopsis-with-inline-argument'].MissingParameters | Should -BeNullOrEmpty + $records['synopsis-with-inline-argument'].OrphanedParameters | + Should -BeNullOrEmpty -Because 'the voided run''s .PARAMETER Other never renders, so crediting it would invent an orphan' + + # LINE-COMMENT help is real help, both as the thing that renders and as the thing that + # suppresses. The old lookup only ever looked at `<#…#>` tokens, so it skipped this run + # entirely and credited the block below -- a block Get-Help never reaches. + $records['linecomment-help-claims-run'].HasHelpBlock | + Should -BeTrue -Because 'measured: `# .SYNOPSIS` on consecutive lines is comment-based help and Get-Help renders it' + $records['linecomment-help-claims-run'].MissingParameters | + Should -Be @('Name') -Because 'the line-comment run claims the help, so the block below it never renders and Name really is undocumented' + $records['linecomment-help-claims-run'].OrphanedParameters | + Should -Be @('Other') -Because 'the .PARAMETER entry that DOES render names a parameter this cmdlet does not declare' + $records['linecomment-continues-run'].MissingParameters | + Should -BeNullOrEmpty -Because 'measured: a line comment in the same run contributes help source, so `# .PARAMETER Name` documents Name' + + # FALL-THROUGH. A region that holds no help run suppresses nothing -- Get-Help carries on + # to the next region. Both directions of that were measured: above to body, and start of + # body to end of body. + $records['invalid-above-falls-through'].HelpAboveFunction | + Should -BeFalse -Because 'the run above the function is not help, so there is nothing above the function to report' + $records['invalid-above-falls-through'].HasHelpBlock | + Should -BeTrue -Because 'measured: Get-Help reads the body exactly as though nothing sat above the function' + $records['invalid-above-falls-through'].MissingParameters | Should -BeNullOrEmpty + $records['invalid-above-falls-through'].OrphanedParameters | Should -BeNullOrEmpty + + $records['ordinary-start-help-at-end'].HasHelpBlock | + Should -BeTrue -Because 'measured: an ordinary run at the start of the body does not claim the help, so the end-of-body block is what renders' + $records['ordinary-start-help-at-end'].MissingParameters | Should -BeNullOrEmpty + + # Delimiters are stripped IN PLACE, not by dropping the lines that carry them. Measured on + # both editions: content on the `<#` line and on the `#>` line both survive into the help. + $records['delimiter-line-content'].HasHelpBlock | + Should -BeTrue -Because 'the .SYNOPSIS shares the opening delimiter''s line and Get-Help still reads it, so dropping that line would void a block that renders' + $records['delimiter-line-content'].MissingParameters | Should -BeNullOrEmpty + $records['delimiter-line-content'].EmptyParameterSections | + Should -BeNullOrEmpty -Because 'the last .PARAMETER body ends on the closing delimiter''s line, so dropping that line would silently empty a section that has text' + + # A malformed directive LATER in the block, which voids all of it. This is the reason the + # run test walks every line rather than just the first: both of these open with a correct + # .SYNOPSIS and a correct .PARAMETER, and Get-Help renders auto-generated syntax help for + # both. 'dotword-prose-voids-block' is the sharp one -- `.NET Core ...` is prose to a + # reader and a malformed directive to Get-Help, because the directive pattern is `\w`. + # (`.\tools\...` in the 'dotted-prose' fixture is the harmless twin: a backslash is not a + # word character, so that line is body text and its block renders. The pair is the point.) + $records['unknown-keyword-later-in-block'].HasHelpBlock | + Should -BeFalse -Because 'measured: one unrecognised directive anywhere in the run voids the whole block, .SYNOPSIS included' + $records['unknown-keyword-later-in-block'].MissingParameters | Should -Be @('Name') + $records['unknown-keyword-later-in-block'].HelpRunDefect | + Should -BeLike '*WIBBLE is not a directive*' + + $records['dotword-prose-voids-block'].HasHelpBlock | + Should -BeFalse -Because 'measured: `.NET Core ...` matches Get-Help''s directive pattern, so it is a malformed directive and not example prose' + $records['dotword-prose-voids-block'].MissingParameters | Should -Be @('Name') + $records['dotted-prose'].HasHelpBlock | + Should -BeTrue -Because 'the twin case: `.\tools\...` is NOT directive-shaped, because a backslash is not a word character, so that block renders' + + # The adjacency BOUNDARY above the function. One blank line is still adjacency and two is + # not, and both sides need a fixture: 'distant-header' looks like it covers this and does + # not, because its last run before the keyword is the line comment rather than the block. + # Widening the bound survived mutation testing until these two landed. + $records['above-blank-line-still-adjacent'].HelpAboveFunction | + Should -BeTrue -Because 'measured: one blank line between the block and the function keyword is still adjacency' + $records['above-blank-line-still-adjacent'].HasHelpBlock | Should -BeFalse + $records['above-blank-line-still-adjacent'].MissingParameters | Should -Be @('Name') + + $records['distant-block-falls-through'].HelpAboveFunction | + Should -BeFalse -Because 'measured: two blank lines break adjacency, so the block above documents the file rather than the function -- the failure direction that keeps a file header from being scored as a cmdlet''s help' + $records['distant-block-falls-through'].HasHelpBlock | + Should -BeTrue -Because 'nothing above the function is help, so Get-Help reads the body' + $records['distant-block-falls-through'].MissingParameters | Should -BeNullOrEmpty + $records['distant-block-falls-through'].OrphanedParameters | Should -BeNullOrEmpty } } From e4ff1ca1ca367c1b5e57107979a5a6fee0885461 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Tue, 25 Aug 2026 14:21:35 -0700 Subject: [PATCH 6/7] test(help): pin the directive character class and the last-run-above rule A scoped review of the help-detector fix found two behaviours the fixture set asserts but does not discriminate: mutating either survives every assertion in the file, so a regression in either would land green. The character class. The existing 'dotword-prose-voids-block' fixture is documented as pinning `\w` against `[A-Za-z]`, and does not -- `.NET` is pure ASCII, so both patterns match it identically and what that fixture actually pins is the argument group. Add 'digit-dotword-voids-block', whose `.EXAMPLE` body opens with `.5 is a fraction ...`: directive-shaped to `\w` alone. Measured on pwsh 7.6.5 and WinPS 5.1, Get-Help renders auto-generated syntax help and no parameter text for it, so narrowing the class would credit a cmdlet whose help does not render at all. Correct the attribution in the two comments that made the wrong claim. Which run above the function counts. Every above-function fixture holds exactly one run up there, so reading the first instead of the last is indistinguishable in all of them -- and in 'distant-header' the first falls through anyway, on adjacency. Add 'last-run-above-function-wins': a file header first, real help last, and a body block as well. Measured on both editions, Get-Help renders the last run, so reading the first would fall through and credit a body block no reader ever sees. Both fixtures were mutation-checked: narrowing the class to `[A-Za-z]` at both sites, and taking `$aboveRuns[0]`, each now fail the suite and passed before. Co-Authored-By: Claude Opus 5 (1M context) --- Tests/PfbHelpCoverage.Tests.ps1 | 75 ++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/Tests/PfbHelpCoverage.Tests.ps1 b/Tests/PfbHelpCoverage.Tests.ps1 index 79cc645..238c44e 100644 --- a/Tests/PfbHelpCoverage.Tests.ps1 +++ b/Tests/PfbHelpCoverage.Tests.ps1 @@ -1251,6 +1251,26 @@ function Get-PfbFixture { [CmdletBinding()] param([Parameter()] [string]$Name) } +'@ + # The one shape that separates `\w` from `[A-Za-z]`, and the reason it needs its own + # fixture: `.NET` above is pure ASCII, so it matches both patterns identically and + # pins only the ARGUMENT group. A digit- or underscore-led dot line is directive-shaped + # to `\w` alone. Measured on both editions -- Get-Help renders auto-generated syntax + # help and no parameter text for this fixture, so narrowing the pattern would credit a + # cmdlet whose help does not render at all. + 'digit-dotword-voids-block' = @' +function Get-PfbFixture { + <# + .SYNOPSIS + A synopsis Get-Help never renders, because of the .EXAMPLE body below. + .PARAMETER Name + The fixture name. + .EXAMPLE + .5 is a fraction and not a path, so this line is a malformed directive + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} '@ # The adjacency BOUNDARY above the function, both sides of it. 'distant-header' never # reaches this: its last run before the keyword is the line comment, so the block above @@ -1288,6 +1308,36 @@ function Get-PfbFixture { [CmdletBinding()] param([Parameter()] [string]$Name) } +'@ + # Which of SEVERAL runs above the function is the one that counts. Every other + # above-function fixture has exactly one run up there, so taking the first instead of + # the last is indistinguishable in all of them -- and in 'distant-header' the first + # falls through anyway, on adjacency. Here the file header is first and the real help + # is last, and the two verdicts differ: measured on both editions, Get-Help renders the + # LAST run, so reading the first would leave the block above unexamined and credit a + # body block Get-Help never reads. + 'last-run-above-function-wins' = @' +<# +.SYNOPSIS + File header for a script, not help for the function below it. +#> + +<# +.SYNOPSIS + The block directly above the function, which Get-Help renders. +.PARAMETER Name + The fixture name. +#> +function Get-PfbFixture { + <# + .SYNOPSIS + A body block Get-Help never reads, because the block above it wins. + .PARAMETER Name + The fixture name. + #> + [CmdletBinding()] + param([Parameter()] [string]$Name) +} '@ 'ordinary-start-help-at-end' = @' function Get-PfbFixture { @@ -1572,9 +1622,15 @@ function Get-PfbFixture { # run test walks every line rather than just the first: both of these open with a correct # .SYNOPSIS and a correct .PARAMETER, and Get-Help renders auto-generated syntax help for # both. 'dotword-prose-voids-block' is the sharp one -- `.NET Core ...` is prose to a - # reader and a malformed directive to Get-Help, because the directive pattern is `\w`. + # reader and a malformed directive to Get-Help. # (`.\tools\...` in the 'dotted-prose' fixture is the harmless twin: a backslash is not a # word character, so that line is body text and its block renders. The pair is the point.) + # + # What separates `.NET` from prose is the DOT plus word characters, which `\w` and + # `[A-Za-z]` agree on -- `NET` is pure ASCII, so this pair does not pin the character class + # and reading it as though it did is how the class went unpinned. What it does pin is the + # argument group: narrowing `(\S.*)` to `(\S+)` is killed here and nowhere else. + # 'digit-dotword-voids-block' is the fixture that pins `\w` itself. $records['unknown-keyword-later-in-block'].HasHelpBlock | Should -BeFalse -Because 'measured: one unrecognised directive anywhere in the run voids the whole block, .SYNOPSIS included' $records['unknown-keyword-later-in-block'].MissingParameters | Should -Be @('Name') @@ -1587,6 +1643,12 @@ function Get-PfbFixture { $records['dotted-prose'].HasHelpBlock | Should -BeTrue -Because 'the twin case: `.\tools\...` is NOT directive-shaped, because a backslash is not a word character, so that block renders' + $records['digit-dotword-voids-block'].HasHelpBlock | + Should -BeFalse -Because 'measured on both editions: `.5 is a fraction ...` is directive-shaped to `\w` and Get-Help renders no help for this cmdlet, so narrowing the class to `[A-Za-z]` would credit a block that does not render' + $records['digit-dotword-voids-block'].MissingParameters | Should -Be @('Name') + $records['digit-dotword-voids-block'].HelpRunDefect | + Should -BeLike '*5 is not a directive*' + # The adjacency BOUNDARY above the function. One blank line is still adjacency and two is # not, and both sides need a fixture: 'distant-header' looks like it covers this and does # not, because its last run before the keyword is the line comment rather than the block. @@ -1602,5 +1664,16 @@ function Get-PfbFixture { Should -BeTrue -Because 'nothing above the function is help, so Get-Help reads the body' $records['distant-block-falls-through'].MissingParameters | Should -BeNullOrEmpty $records['distant-block-falls-through'].OrphanedParameters | Should -BeNullOrEmpty + + # WHICH run above the function is examined, when there is more than one. Both fixtures + # above hold a single run up there, so they cannot tell the last from the first; this one + # puts a file header first and the real help last. Measured on both editions: Get-Help + # renders the LAST run's synopsis and its parameter text, so examining the first would + # find a non-adjacent header, fall through, and credit the body block instead. + $records['last-run-above-function-wins'].HelpAboveFunction | + Should -BeTrue -Because 'measured: the run nearest the function keyword is the one Get-Help renders, header or not' + $records['last-run-above-function-wins'].HasHelpBlock | + Should -BeFalse -Because 'the help that renders sits above the function, so the body block is not what a reader sees and must not be scored as though it were' + $records['last-run-above-function-wins'].MissingParameters | Should -Be @('Name') } } From 0a81619d3faf20e8ad67dffde48e829857735804 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Tue, 25 Aug 2026 15:12:03 -0700 Subject: [PATCH 7/7] chore(reports): regenerate the drift report for the moved param block Adding the missing help block to Update-PfbSupport.ps1 pushed its param() block from line 5 to line 24, and the drift report records that line as the target of each finding. Six `paramBlockLine` values, one per finding against that file; no endpoint, parameter or enum verdict moves. Regenerated through scripts/Assert-PfbDerivedArtifacts.ps1 -UpdateCommitted rather than by running the generator bare, so the output is the one the CI gate compares against: the gate stages exactly the 29 pinned spec versions, while Build-PfbApiDriftReport.ps1 run on its own records whatever the local tools/specs/ cache happens to hold. Co-Authored-By: Claude Opus 5 (1M context) --- Reports/PfbApiDriftReport.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Reports/PfbApiDriftReport.json b/Reports/PfbApiDriftReport.json index 3a2ba92..eb67e4b 100644 --- a/Reports/PfbApiDriftReport.json +++ b/Reports/PfbApiDriftReport.json @@ -7513,7 +7513,7 @@ "enumStatus": "no-spec-enum-found", "target": { "file": "Public/Support/Update-PfbSupport.ps1", - "paramBlockLine": 5, + "paramBlockLine": 24, "payloadVariable": "Attributes", "assignmentStyle": "attributesOnly", "hasAttributes": true @@ -7530,7 +7530,7 @@ "enumStatus": "no-spec-enum-found", "target": { "file": "Public/Support/Update-PfbSupport.ps1", - "paramBlockLine": 5, + "paramBlockLine": 24, "payloadVariable": "Attributes", "assignmentStyle": "attributesOnly", "hasAttributes": true @@ -7547,7 +7547,7 @@ "enumStatus": "no-spec-enum-found", "target": { "file": "Public/Support/Update-PfbSupport.ps1", - "paramBlockLine": 5, + "paramBlockLine": 24, "payloadVariable": "Attributes", "assignmentStyle": "attributesOnly", "hasAttributes": true @@ -7564,7 +7564,7 @@ "enumStatus": "no-spec-enum-found", "target": { "file": "Public/Support/Update-PfbSupport.ps1", - "paramBlockLine": 5, + "paramBlockLine": 24, "payloadVariable": "Attributes", "assignmentStyle": "attributesOnly", "hasAttributes": true @@ -7581,7 +7581,7 @@ "enumStatus": "no-spec-enum-found", "target": { "file": "Public/Support/Update-PfbSupport.ps1", - "paramBlockLine": 5, + "paramBlockLine": 24, "payloadVariable": "Attributes", "assignmentStyle": "attributesOnly", "hasAttributes": true @@ -7598,7 +7598,7 @@ "enumStatus": "no-spec-enum-found", "target": { "file": "Public/Support/Update-PfbSupport.ps1", - "paramBlockLine": 5, + "paramBlockLine": 24, "payloadVariable": "Attributes", "assignmentStyle": "attributesOnly", "hasAttributes": true