Tests - Enforce the Stop-Function flow-control invariants over all commands

[andreasjordan]

Implements the enforcement and documentation agreed in #10655: AST enforcement for rules 1 and 3, the other rules as guidance.

What it adds

• tests/dbatools.CommandStructure.ps1: the analyzer Get-DbaCommandStructureFinding, which needs no SQL Server and does not load the module.
• "command structure" Describe in tests/dbatools.Tests.ps1, next to the test-file checks (tag Compliance). It parses the 887 files in public/ and private/functions/ in about 10 seconds and reports file:line function - message:
  • every command file parses;
  • every Stop-Function parameter binds;
  • rule 1: a Stop-Function that can continue has a loop or switch to continue in the same function or scriptblock, and -ContinueLabel names one of them;
  • rule 3: a begin block that can set the interrupt flag is followed by if (Test-FunctionInterrupt) { return } as the first statement of process.
• "command structure analyzer" Describe: 22 positive and negative fixtures for the analyzer itself.
• CLAUDE.md: a COMMAND INVARIANTS section with rules 1-6. Rules 1 and 3 are marked as enforced, rule 2 and rules 4-6 are guidance, in the wording of the review on the issue. It is linked from a new section in tests/CLAUDE.md.

How the review points are handled

• Loops and switch both count as targets. -ContinueLabel must match the label of an enclosing loop or switch.
• The target search stops at a function or scriptblock boundary. A loop around a helper definition or a ForEach-Object callback does not count; a callback with its own loop does.
• -SilentlyContinue is not treated like -Continue.
  • For rule 1, either switch needs a target: -Continue continues in the default mode, -SilentlyContinue under EnableException.
  • For rule 3, only a statically true -Continue is exempt. -SilentlyContinue alone sets the flag in the default mode, so it requires the guard.
• Scriptblocks are classified by how they run, not excluded wholesale. Measured on 5.1 and 7.6, a Stop-Function flag set in begin reaches the command's process for a direct call, ForEach-Object, Where-Object, . { } and .ForEach()/.Where(). It does not for & { }, Invoke-Command, .Invoke() or a helper function. Any other invocation is reported for review.
• Unresolved cases are not compliant. -Continue:$variable, a splat that is not a hashtable literal assigned earlier in the same function, and a -ContinueLabel that is not a constant all count as "may continue". A parameter that binds to nothing (checked by name, alias and unique prefix, like PowerShell) is reported.
• Parse errors fail the check.
• The fixtures cover the cases asked for:
  • disabled and variable switches (-Continue:$false, -Continue:$variable);
  • resolved and unresolved splats;
  • nested helpers and callbacks, dot-sourced and new-scope scriptblocks;
  • matching, missing and dynamic labels;
  • missing and misplaced guards, and a begin stop without a process block;
  • aliases, prefixes, case, and -EnableException taking a value.
• Dynamic-binding exceptions are a list in the Describe: file, helper function, exact number of sites and the reason (the sites triaged in Stop-Function -Continue without an enclosing loop escapes the command and corrupts the caller #10638; Measure-DbaDiskSpaceRequirement - Return ? for unreadable mount points instead of skipping rows #10756 and Backup-DbaDbCertificate - Stop before exporting to a path the instance cannot access #10757 restructure the helpers of Measure-DbaDiskSpaceRequirement and Backup-DbaDbCertificate, so 17 sites in 8 helpers remain). A new site in the same helper, or one that is gone, fails the check. Behavioral tests for these callers are not part of this PR.

Findings on development, fixed in their own PRs

The first run found six defects that the earlier text sweeps missed. Each is fixed in its own draft PR, lab-tested on both editions:

• Remove-DbaAgentJobStep - Stop ending the calling script when a job or step is missing #10750 Remove-DbaAgentJobStep, Disable-DbaReplPublishing - Stop ending the calling script for an instance that is not a publisher #10751 Disable-DbaReplPublishing, Set-DbaSpConfigure - Stop ending the calling script when the server refuses a change #10752 Set-DbaSpConfigure: -ContinueLabel main with no loop carrying that label. Measured on 5.1 and 7.6, a labeled continue that matches nothing ends the whole calling script silently with exit code 0. Against the old commands, their new tests ended the entire test run.
• Export-DbaExecutionPlan - Warn when the plan query fails and export every -InputObject plan #10753 Export-DbaExecutionPlan: -ErroRecord binds to nothing, so the catch failed with a parameter binding error instead of warning.
• Invoke-DbaAdvancedRestore - Guard process after a stop in begin #10754 Invoke-DbaAdvancedRestore: begin can stop, process had no guard (latent).
• Measure-DbaDiskSpaceRequirement - Return ? for unreadable mount points instead of skipping rows #10756 Measure-DbaDiskSpaceRequirement: its mount point helpers relied on a dynamic continue that skipped rows; they now warn and return ?. Its two entries are dropped from the exception list here.
• Backup-DbaDbCertificate - Stop before exporting to a path the instance cannot access #10757 Backup-DbaDbCertificate: the path check had no return, so the export was attempted anyway; every stop in its helper now returns instead of relying on a dynamic continue. Its entry is dropped from the exception list here.

This PR stays red until those seven merge. With all seven merged onto this branch locally, the two Describes pass 26/26 on Windows PowerShell 5.1 and PowerShell 7.6.3. The existing style and test-file-structure checks pass with the new files.

Closes #10655

---

This text was created by Claude and reviewed by Andreas Jordan.

🤖 Generated with Claude Code

[potatoqualitee]

Three blocking false negatives let commands violate the very flow-control invariants this PR is intended to enforce:

1. ests/dbatools.CommandStructure.ps1:155-166 resolves a splat from the last lexical variable assignment anywhere earlier in the function. It ignores later member mutation and includes assignments in nested or nonexecuted paths. For example, $splatStop = @{ Continue = False }; .Continue = True; Stop-Function @splatStop produces zero findings, yet the runtime continue can escape the caller's loop. Resolve only demonstrably reaching definitions in the actual execution scope, detect mutations, and treat uncertainty as unresolved; add negative fixtures for mutation, uncalled nested helpers, and conditional assignments.

2. Line 301 exempts every statically true -Continue from rule 3. With -EnableException True, real Stop-Function sets the interrupt flag and throws before any continue; if begin catches that exception, process runs unless it has the guard. A begin-block ry { Stop-Function -Continue -EnableException True } catch {} currently produces zero findings while process work executes with the flag set. Require the guard when that throwing path can resume, or report the path for review; add a caught-throw fixture.

3. Lines 280-291 recognize a guard from only the command name and return body. if (Test-FunctionInterrupt > ) { return } is accepted, but the true value is discarded, the condition is false, and process work runs. Unsupported arguments are likewise accepted. Reject success-output redirection and arguments when recognizing the guard, and add negative fixtures.

These are material misleading-compliance failures: the new analyzer and green test suite certify unsafe commands. All three paths were independently confirmed from the exact analyzer and the real Stop-Function/Test-FunctionInterrupt semantics.

[andreasjordan]

Thanks, all three confirmed: each of your examples produced zero findings. Fixed in f1e5d0f and 599a710.

1. Splat resolution. A splat now counts as resolved only from the definition that certainly reaches the call:

• the last $name = @{ ... } before the call in the same function;
• at statement level, in a block that encloses the call.

It is unresolved when any of these happens:

• the hashtable is written between that definition and the call, or in a loop or scriptblock around the call. Writes are key assignments (.Key =, ["Key"] =), method calls, [ref], ++/--, and handing the hashtable to a command or to another variable;
• a nested function or scriptblock writes it;
• Set-/New-/Remove-/Clear-/Get-Variable names it;
• the function uses Invoke-Expression;
• the name appears as an argument, as in -OutVariable.

Unresolved splats report an "Arguments" finding. Every switch then counts as unknown, including EnableException, so they also count for rules 1 and 3.

Reusing one splat name for several literals, each directly before its own call, stays resolved (Install-DbaAgentAdminAlert does that).

2. Caught throw in begin. A -Continue call in begin now counts as able to set the interrupt flag when all of these hold:

• EnableException is not literally $false. Not passing it, or passing -EnableException:$EnableException, counts, because it defaults to the caller's value;
• -SilentlyContinue is not passed as well;
• the call sits in the body of a try that has a catch, or under a trap.

Rule 3 then requires the guard. The CLAUDE.md text of rule 3 says so now.

3. Guard recognition. Test-FunctionInterrupt is only accepted as a guard with no arguments, no redirection and no &/. invocation operator.

Fixtures. Negative:

• your three examples;
• index assignment, .Add(), [ref], ++, Set-Variable, and an alias that is changed;
• a conditional reassignment;
• an assignment only in an uncalled nested helper, and a nested helper that changes the splat;
• a write later in the loop;
• a caught throw with $true, with :$EnableException, and under trap;
• a guard with > $null, with *> $null, and with an argument.

Positive: the reused splat name, and a caught -Continue with $false or with -SilentlyContinue. With the previous analyzer, exactly the new negative fixtures fail.

What it found. The stricter splat rule found one site that the old one had certified: the private helper Test-ElevationRequirement added Continue to its splat by index before calling Stop-Function. That is your example 1 in production code.

• It now builds the splat as one literal. Stop-Function only tests these switches for truth, so the behavior is the same.
• It is on the allowlist as a deliberate forwarder of the caller's -Continue. 20 of its 22 -Continue callers sit in a loop of their command, and Invoke-DbaPfRelog is already on the allowlist.
• The remaining caller, Install-DbaSqlPackage line 164, calls it with -Continue outside any loop: a real escape. That fix goes into a separate PR.

Lab: both "command structure" Describes 34/34 on Windows PowerShell 5.1 and PowerShell 7.6.3.

---

This text was created by Claude and reviewed by Andreas Jordan.

[potatoqualitee]

Reviewed the exact current head. The three prior analyzer false negatives are addressed: uncertain/mutated splats are no longer certified, caught EnableException paths from begin require the process guard, and redirected/argument-bearing Test-FunctionInterrupt calls no longer count as guards. The new fixtures cover those failure modes and the current compliance CI is green. I found no remaining material defect.

[potatoqualitee]

very nice 💯