-
Notifications
You must be signed in to change notification settings - Fork 399
Added PSUseFullyQualifiedCmdletNames rule with fix capabilities #2122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new PowerShell Script Analyzer rule PSUseFullQualifiedCmdletNames
that enforces the use of fully qualified cmdlet names (e.g., ModuleName\CmdletName
) instead of aliases or unqualified names to improve script reliability and prevent ambiguity.
- Implements diagnostic rule with automatic fix capabilities for replacing aliases and unqualified cmdlets
- Adds caching mechanism for command resolution to improve performance
- Provides localized error messages and correction descriptions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
Rules/UseFullyQualifiedCmdletNames.cs | Core implementation of the new diagnostic rule with command resolution and fix suggestions |
Rules/Strings.resx | Localized string resources for error messages and rule descriptions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
var extent = commandAst.CommandElements[0].Extent; | ||
|
||
bool isAlias = commandName != fullyQualifiedName.Split('\\')[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for determining if a command is an alias is incorrect. This will incorrectly identify unqualified cmdlets as aliases when the command name matches the actual cmdlet name. Consider checking the resolved command type instead: bool isAlias = resolvedCommand.CommandType == CommandTypes.Alias;
bool isAlias = commandName != fullyQualifiedName.Split('\\')[1]; |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not following..
|
||
var extent = commandAst.CommandElements[0].Extent; | ||
|
||
bool isAlias = commandName != fullyQualifiedName.Split('\\')[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Split('\\')[1]
operation is performed for every command analysis. Since the actual cmdlet name is already available from the resolution logic above (line 99), consider storing it in a variable to avoid redundant string operations.
bool isAlias = commandName != fullyQualifiedName.Split('\\')[1]; | |
else | |
{ | |
// Extract actualCmdletName from the cached fullyQualifiedName | |
int idx = fullyQualifiedName.IndexOf('\\'); | |
actualCmdletName = (idx >= 0 && idx < fullyQualifiedName.Length - 1) | |
? fullyQualifiedName.Substring(idx + 1) | |
: fullyQualifiedName; | |
} | |
var extent = commandAst.CommandElements[0].Extent; | |
bool isAlias = commandName != actualCmdletName; |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
@microsoft-github-policy-service agree I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer. @microsoft-github-policy-service agree company="Microsoft" |
@microsoft-github-policy-service agree I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer. @microsoft-github-policy-service agree company="Microsoft" |
👋 Hey @genXdev, could you please add your tests and docs to the PR for your new rule? If you're still working on it, please title the PR as WIP and mark as draft. My initial thoughts on this:
|
I have committed them;
Whatever you think is best.
If this still desirable, I'll add them, let me know.
Haven't tested that, for fixing damage, I only enabled my new rule with -Fix Maybe not the place to mention it, but analyzing 100+ script files at once, I sometimes see 'Collection modified' concurrency exceptions. |
Also added 'IgnoredModules' parameter and updated tests and docs accordingly |
PR Summary
Add new diagnostic rule
PSUseFullyQualifiedCmdletNames
to replace aliases and unqualified cmdlet names with fully qualified versions (e.g.,ModuleName\CmdletName
).This rule addresses a common pain point in PowerShell scripting where cmdlet names without module prefixes can lead to ambiguity, especially in environments with multiple modules exporting similarly named cmdlets. By enforcing fully qualified names, the rule provides the following benefits:
This change directly relates to PowerShell/PSScriptAnalyzer#2123, where the PowerShell extension for VS Code has been reported to unexpectedly remove module prefixes (e.g., converting
MicrosoftTeams\Get-CsLisCivicAddress
toGet-CsLisCivicAddress
) during code formatting, potentially introducing ambiguities and runtime issues in scripts. This new rule enables users to automatically add or restore fully qualified cmdlet names during analysis or formatting, helping to repair the damage caused by such removals and promoting safer, more explicit scripting practices.Implemented in
Rules/UseFullyQualifiedCmdletNames.cs
, with accompanying tests in the test suite to verify replacement logic for aliases (e.g.,ls
→Microsoft.PowerShell.Management\Get-ChildItem
) and unqualified cmdlets.PR Checklist
.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
to the beginning of the title and remove the prefix when the PR is ready.