Repository navigation
Distinguish script-defined functions from DSC resources - #2297
Christoph Bergmeister (bergmeister) with Copilot wants to merge 13 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 <223556219+Copilot@users.noreply.github.com>
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 <9250262+bergmeister@users.noreply.github.com>
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 <9250262+bergmeister@users.noreply.github.com>
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 <9250262+bergmeister@users.noreply.github.com>
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 <9250262+bergmeister@users.noreply.github.com>
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Mutually recursive scope checks can stall analysis on nested callbacks, while unresolved function-visibility and scope-boundary errors cause missed diagnostics and behavior-changing corrections.
7 open issues
Prevent exponential recursion in scope visibility checks · New Preserve alias precedence in callback scope detection · New Respect declaration order when resolving same-scope functions · New Resolve dot-sourced declarations before classifying callbacks · New Treat NoNewScope callbacks as caller-scoped · New Recognize standalone DSC property script blocks · New Account for parameter binding when identifying callback blocks · New
4 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved scope, execution-order, and callback-classification errors in AvoidAlias suppress expected diagnostics and can incorrectly rewrite valid function calls.
5 open issues
7 resolved since last review
Prevent exponential recursion in scope visibility checks Account for parameter binding when identifying callback blocks Recognize standalone DSC property script blocks Treat NoNewScope callbacks as caller-scoped Resolve dot-sourced declarations before classifying callbacks Respect declaration order when resolving same-scope functions Preserve alias precedence in callback scope detection
Review effort: Balanced
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved positional-argument handling, callback classification, and execution-order bugs in AvoidAlias suppress expected Get-Verb diagnostics and cause the formatter to replace valid function calls.
2 open issues
5 resolved since last review
Review effort: Balanced
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Human assessment is needed because interacting scope, execution-order, DSC-property, and runspace-boundary cases can incorrectly suppress diagnostics or cause erroneous formatter changes.
No open issues 🎉
2 resolved since last review
Review effort: Balanced


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