Skip to content

Distinguish script-defined functions from DSC resources - #2297

Draft
Christoph Bergmeister (bergmeister) with Copilot wants to merge 39 commits into
copilot/fix-auto-correct-aliasesfrom
fix/dsc-script-local-command
Draft

Christoph Bergmeister (bergmeister) with Copilot wants to merge 39 commits into
copilot/fix-auto-correct-aliasesfrom
fix/dsc-script-local-command

Conversation

Copilot AI commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

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.

  • Recognize function declarations in enclosing script-block scopes, case-insensitively, without searching unrelated nested scopes.
  • Use that information consistently for resource declarations and their same-line/following-line bodies. Do not apply implicit Get- prefix corrections to script-defined function calls; preserve explicit alias handling.
  • Cover script-level and configuration-level functions, both brace layouts, unrelated scopes, and formatter behavior. Document the distinction.

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

  • PR has a meaningful title
  • Summarized changes
  • Change is not breaking
  • All changed source files have the correct copyright header
  • Added regression tests and updated documentation
  • This PR is ready to merge and is not Work in Progress (opened as a draft)

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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Singleton subexpressions are not unwrapped, causing incorrect function-scope resolution, erroneous command rewrites, and remote-command misclassification.

1 open issue
5 resolved since last review

Review effort: Balanced

Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved execution-order and scope-analysis errors cause valid function calls to be incorrectly rewritten as Get-Verb and required alias diagnostics to be suppressed.

4 open issues

Review effort: Balanced

Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread Rules/AvoidAlias.cs Resolved
Copilot AI and others added 2 commits November 5, 2026 18:28
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved scope-isolation and execution-order errors in AvoidAlias can suppress required diagnostics or incorrectly rewrite valid function calls.

4 open issues

Review effort: Balanced

Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unwrapping stops at [scriptblock] casts, causing incorrect scope and invocation classification that rewrites valid Verb calls to Get-Verb.

1 open issue

Review effort: Balanced

Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Named-argument binding can cause incorrect alias rewrites, while oscillating scope resolution makes diagnostics depend on unrelated function declarations.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved execution-order and reachability errors in AvoidAlias cause valid function calls to be rewritten and required implicit-alias diagnostics to be suppressed.

6 open issues

Review effort: Balanced

Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

🟡 Changes recommended...

Addressed all six review findings in d16253b. The targeted AvoidUsingAlias tests pass (140 passed).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread Rules/AvoidAlias.cs Resolved
Comment thread Rules/AvoidAlias.cs Resolved
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Two critical pipeline callback-ordering errors can discard visible function declarations and cause the formatter to incorrectly replace script-defined Verb calls with Get-Verb.

2 open issues
2 resolved since last review

Review effort: Balanced

Comment thread Rules/AvoidAlias.cs Outdated
Comment on lines +617 to +619
return firstPhase < secondPhase
|| (firstPhase == secondPhase
&& firstElementIndex < secondElementIndex);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 36e9064. Upstream End callback output is now considered visible to downstream Process callbacks, with regression coverage.

Comment thread Rules/AvoidAlias.cs
Comment on lines +965 to +967
else if (IsParameter(parameterName, "Process"))
{
callbacks.Add(Tuple.Create(commandAst, 1, depth));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 36e9064. Positional ForEach-Object blocks now account for the implicit Begin phase when explicit Begin/End bindings are absent, with regression coverage.

Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants