Repository navigation
Distinguish script-defined functions from DSC resources - #2297
Christoph Bergmeister (bergmeister) with Copilot wants to merge 36 commits into
Conversation
Resolve visible function declarations from the AST when classifying DSC resource syntax and checking implicit Get-prefix aliases. Preserve analysis of executable function arguments for both brace layouts. Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
Scoped function declarations are mishandled, causing incorrect alias rewrites and suppressed diagnostics, while repeated scope scans introduce quadratic analysis overhead.
2 open issues
What changed in this PR
This follow-up to #2270 distinguishes script-defined function calls from DSC resource declarations so alias analysis preserves function names while checking commands in their script-block arguments.
Changes:
- Adds case-insensitive function lookup in enclosing script-block scopes and reuses it in DSC classification.
- Adds regression coverage for function scope, brace layouts, unrelated scopes, and formatter behavior.
- Documents the distinction between functions, DSC resources, and implicit aliases.
| File | Description |
|---|---|
| Tests/Rules/AvoidUsingAlias.tests.ps1 | Adds function-detection and formatter regression tests. |
| Rules/AvoidAlias.cs | Uses script-defined function detection in alias and DSC handling. |
| docs/Rules/AvoidUsingCmdletAliases.md | Explains function and DSC resource exclusions from implicit alias correction. |
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
Scope handling still changes command resolution for dot-sourced private functions and incorrectly treats script/global declarations in unexecuted nested functions as visible, causing missed diagnostics and unsafe corrections.
2 open issues
2 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
Dot-sourced scope handling still misindexes declarations and incorrectly skips script-block argument scopes, causing both erroneous alias corrections and missed diagnostics.
2 open issues
2 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
The argument-count restriction incorrectly treats parameterized dot-sourced blocks as child scopes, causing erroneous alias formatting and missed declarations in the caller’s scope.
1 open issue
2 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🔵 Needs a closer look
Human assessment is needed because interacting callback-scope, execution-order, and remote-target rules can suppress valid alias diagnostics or cause incorrect command rewrites.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved scope and execution-order defects in AvoidAlias cause incorrect Get-Verb rewrites and missed diagnostics across callbacks, nested blocks, and job initialization.
7 open issues
Resolve aliases to local functions for callback lookup · New Include synchronous callbacks in invocation-order resolution · New Compare phases at the nearest shared execution block · New Exclude other caller-scoped callbacks from same-pipeline lookup · New Resolve RemainingScripts declarations by execution phase · New Traverse callbacks for explicit script and global declarations · New Apply initialization-to-job scope mapping before returning · New
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
Module-qualified calls can incorrectly resolve to local functions and suppress diagnostics, while incorrect callback ordering can rewrite commands despite prior function declarations.
2 open issues
7 resolved since last review
Apply initialization-to-job scope mapping before returning Traverse callbacks for explicit script and global declarations Resolve RemainingScripts declarations by execution phase Exclude other caller-scoped callbacks from same-pipeline lookup Compare phases at the nearest shared execution block Include synchronous callbacks in invocation-order resolution Resolve aliases to local functions for callback lookup
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🔵 Needs a closer look
Mixed named and positional callback binding is handled incorrectly, causing a valid function call to be misidentified as an alias and rewritten to Get-Verb.
No open issues 🎉
2 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved scope and execution-order handling errors in AvoidAlias produce incorrect Get-Verb rewrites and suppress valid diagnostics across local, remote, and callback script blocks.
5 open issues
Recognize the clean block in execution-order checks · New Include synchronous local Invoke-Command blocks in scope resolution · New Traverse subexpression wrappers during callback resolution · New Treat SSHConnection as a remote Invoke-Command target · New Only treat literal script-block arguments as local callbacks · New
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🔵 Needs a closer look
Human assessment is needed because execution-order and scope-isolation edge cases can suppress valid alias diagnostics or produce incorrect formatter replacements.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
Scope-boundary searches incorrectly isolate nested blocks in DSC callbacks and background pipelines, hiding enclosing local functions and causing erroneous alias rewrites.
2 open issues
4 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
The ancestor walk incorrectly treats a nested hashtable callback as an isolated DSC resource callback, causing a false Get-Verb suggestion; it needs to stop at an intervening ScriptBlockAst and gain regression coverage.
1 open issue
2 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🔵 Needs a closer look
The scope traversal incorrectly promotes global: and script: function declarations from background jobs into the foreground scope, suppressing required diagnostics for implicit Get-Verb calls.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
The pipeline check incorrectly rejects function declarations that execute before later calls within the same pipeline element, causing valid calls such as Verb to be rewritten to Get-Verb.
1 open issue
4 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🔵 Needs a closer look
Human assessment is needed because subtle scope and execution-order interactions involving scriptblock casts, eager arguments, and background callbacks can cause incorrect alias resolution.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <[email protected]>
There was a problem hiding this comment.
🔵 Needs a closer look
Positional binding fails to exclude parameters supplied by name, causing incorrect execution-order analysis and an erroneous rewrite of Verb to Get-Verb in mixed named/positional commands.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved callback-binding and execution-flow errors produce incorrect alias diagnostics by misclassifying positional callbacks and indexing declarations from nonexecuting iterators or discarded script blocks.
2 open issues
Review effort: Balanced
| if (current.Parent is ConvertExpressionAst convertExpression | ||
| && convertExpression.Child == current) |
There was a problem hiding this comment.
Fixed in 03be28b: void-cast script blocks are no longer treated as callbacks. Added a regression test; targeted tests pass.
Co-authored-by: bergmeister <[email protected]>

PR Summary
Follow-up to #2270, targeting its branch rather than
main.The review finding is reproducible: a resource-shaped call to a script-defined function is treated as a DSC declaration because command discovery uses a separate runspace. Commands inside its script block arguments are then incorrectly suppressed. Both new regression cases returned zero diagnostics before the fix instead of the two expected diagnostics.
Get-prefix corrections to script-defined function calls; preserve explicit alias handling.Validation: .NET 8 and .NET Framework 4.6.2 builds succeeded. Alias tests: 23 passed, 2 Windows-only tests skipped on Linux. Formatter integration tests: 10 passed. Executed using PowerShell 7.4.13 and Pester 5.7.1; the configured Pester 6.0.1 package was unavailable from the accessible mirror.
PR Checklist