Added PSUseFullyQualifiedCmdletNames rule with fix capabilities - #2122
René Vaessen (genXdev) wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
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.
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]; |
|
|
||
| var extent = commandAst.CommandElements[0].Extent; | ||
|
|
||
| bool isAlias = commandName != fullyQualifiedName.Split('\\')[1]; |
There was a problem hiding this comment.
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; |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@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 René Vaessen (@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 |
|
Sorry it's taken so long. Agree to not enable it by default but otherwise happy to have it. I know some module owners do this for performance reasons and replace commands with the full version as part of their build process. René Vaessen (@genXdev) can you resolve the merge conflict please, I will then greenlight the running of the CI test suite |
Hi Christoph Bergmeister (@bergmeister), I've resolved the merge conflict. The conflict was in Strings.resx where my UseFullyQualifiedCmdletNames resource entries overlapped with the new AvoidReservedWordsAsFunctionNames entries from main. I kept both sets of changes. Merge commit: f6d123d The branch should now be ready for CI tests to run. |
|
René Vaessen (@genXdev) There was one test failure, can you look into resolving it please. In the meantime I updated branch again and kicked off new test run, which shows failure still happens there as well |
|
René Vaessen (@genXdev) kind reminder and please resolve merge conflict again please and review new Copilot comments, sorry for delay |
There was a problem hiding this comment.
🟡 Changes recommended
The fixer can change calls to locally declared functions, and several diagnostics, tests, and documentation details are inaccurate.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
Rules/UseFullyQualifiedCmdletNames.cs:151
- Alias detection is case-sensitive, so an ordinary differently-cased cmdlet such as
get-commandis reported as an alias because its canonical name isGet-Command. Preserve whether the resolved command was an alias, or at least compare these names case-insensitively.
bool isAlias = commandName != fullyQualifiedName.Split('\\')[1];
- Files reviewed: 5/5 changed files
- Comments generated: 6
- Review effort level: Balanced
|
|
||
| if (!resolutionCache.TryGetValue(commandName, out string fullyQualifiedName)) | ||
| { | ||
| var resolvedCommand = ResolveCommand(commandName); |
| string message = string.Format( | ||
| CultureInfo.CurrentCulture, | ||
| isAlias ? Strings.UseFullyQualifiedCmdletNamesAliasError : Strings.UseFullyQualifiedCmdletNamesCommandError, | ||
| commandName, |
| It "ignores native commands" { | ||
| $scriptDefinition = @' | ||
| where.exe notepad | ||
| cmd /c dir | ||
| '@ | ||
| $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $violationName | ||
| $violations.Count | Should -Be 0 | ||
| } |
| <value>Use Fully Qualified Cmdlet Names</value> | ||
| </data> | ||
| <data name="UseFullyQualifiedCmdletNamesDescription" xml:space="preserve"> | ||
| <value>Cmdlets should be called using their fully qualified names instead of aliases or abbreviated forms.</value> |
| | [UseConsistentIndentation](./UseConsistentIndentation.md) | Warning | No | Yes | | ||
| | [UseConsistentWhitespace](./UseConsistentWhitespace.md) | Warning | No | Yes | | ||
| | [UseCorrectCasing](./UseCorrectCasing.md) | Information | No | Yes | | ||
| | [UseFullyQualifiedCmdletNames](./UseFullyQualifiedCmdletNames.md) | Warning | Yes | | |
|
|
||
| - **Automatic Import**: If the specified module isn't already loaded, PowerShell will automatically import it when the cmdlet is called | ||
| - **Explicit Dependencies**: The script clearly declares which modules it depends on without requiring manual `Import-Module` calls | ||
| - **Version Control**: Helps ensure the correct module version is loaded, especially when multiple versions are installed |
PR Summary
Add new diagnostic rule
PSUseFullyQualifiedCmdletNamesto 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-CsLisCivicAddresstoGet-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,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.